Skip to content

wasm: refactor common codes around configurations#15210

Merged
mattklein123 merged 7 commits intoenvoyproxy:mainfrom
mathetake:wasm-config
Mar 2, 2021
Merged

wasm: refactor common codes around configurations#15210
mattklein123 merged 7 commits intoenvoyproxy:mainfrom
mathetake:wasm-config

Conversation

@mathetake
Copy link
Member

@mathetake mathetake commented Feb 26, 2021

Refactored wasm common codes around configurations by introducing WasmConfig class which performs conversion of restriction configs (and other configs in the future). Resolved the comment istio/proxy#3202 (comment).

@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #15210 was opened by mathetake.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Feb 26, 2021
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
@mathetake mathetake marked this pull request as ready for review February 27, 2021 01:30
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
@PiotrSikora
Copy link
Contributor

@mathetake it looks that you forgot to add some files.

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
@mathetake
Copy link
Member Author

sorry, I missed them. I just Added.

@mathetake
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15210 (comment) was created by @mathetake.

see: more, trace.

@mathetake
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15210 (comment) was created by @mathetake.

see: more, trace.

@mathetake
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #15210 (comment) was created by @mathetake.

see: more, trace.

@PiotrSikora
Copy link
Contributor

@mathetake that failure is deterministic (coverage is too low) and I believe unrelated to this PR. Merging main might fix it.

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
@mathetake
Copy link
Member Author

mathetake commented Mar 1, 2021

Merging main might fix it.

Unfortunately the branch is already up-to-date, so I believe something is broken on main branch, and that's unrelated to this PR since the same happens on other PRs.

Copy link
Contributor

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@mathetake
Copy link
Member Author

coverage failure will be fixed in #15238

alyssawilk pushed a commit that referenced this pull request Mar 1, 2021
)

The coverage on source/server recently has been flaky between 94.4 and 95.1. What's worse is
this is somehow deterministic, i.e. on some commits, retesting would not work to go green.

I have encountered this several times in a last few weeks, for example, #15210 this is clearly unrelated to this coverage issue. You can see the same flake is happening to other PRs.

I see that the root cause sits in source/server/connection_handler_impl.cc, but I would like to introduce this change as a temporary workaround.

Signed-off-by: Takeshi Yoneda takeshi@tetrate.io
@mattklein123 mattklein123 merged commit 3f54cd8 into envoyproxy:main Mar 2, 2021
@mathetake mathetake deleted the wasm-config branch March 2, 2021 22:14
alyssawilk pushed a commit that referenced this pull request Mar 5, 2021
Added a missing unittest in #15210

resolves #15210 (comment)

Signed-off-by: Takeshi Yoneda takeshi@tetrate.io

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
PiotrSikora added a commit to PiotrSikora/envoy that referenced this pull request Mar 23, 2021
Broken during refactor in envoyproxy#15210.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
lizan pushed a commit that referenced this pull request Mar 23, 2021
Broken during refactor in #15210.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deps Approval required for changes to Envoy's external dependencies

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants