fix: RPC mode does not honor timeout #1230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
requested review from
beeme1mr,
Kavindu-Dodan,
thisthat and
toddbaert
aepfli
added a commit
to open-feature/flagd-testbed
that referenced
this pull request
Feb 19, 2025
relates: open-feature/java-sdk-contrib#1230 Signed-off-by: Simon Schrottner <[email protected]>
aepfli
added a commit
to open-feature/flagd-testbed
that referenced
this pull request
Feb 19, 2025
relates: open-feature/java-sdk-contrib#1230 Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
fix/RPC_mode_not_using_deadline
branch
from
50cb01b to
9240d70
Compare
aepfli
added a commit
to open-feature/flagd-testbed
that referenced
this pull request
Feb 19, 2025
relates: open-feature/java-sdk-contrib#1230 Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
fix/RPC_mode_not_using_deadline
branch
2 times, most recently
from
ee7ed54 to
7deaff5
Compare
aepfli
commented
Feb 19, 2025
...agd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java
Show resolved
Hide resolved
aepfli
force-pushed
the
fix/RPC_mode_not_using_deadline
branch
from
7deaff5 to
506c587
Compare
aepfli
added a commit
to open-feature/flagd-testbed
that referenced
this pull request
Feb 19, 2025
relates: open-feature/java-sdk-contrib#1230 Signed-off-by: Simon Schrottner <[email protected]>
|
@aepfli while we are here, can we add a deadline to the getSyncMetadata call in the in-process as well? |
I tried to add it, but it breaks our reconnection tests in a strange way. If we want to add it, I think it is worth to explore this separately. I also opened an issue with discussion points to start with. Eg. The sync metadata does not have to be part of our loop, we could do this everytime the channel is reestablished. Anyways I think we should discuss this, and define this more precisely, as this could be also the way to go for other languages See: #1231 |
aepfli
added a commit
to open-feature/flagd-testbed
that referenced
this pull request
Feb 19, 2025
relates: open-feature/java-sdk-contrib#1230 Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
fix/RPC_mode_not_using_deadline
branch
from
506c587 to
416060f
Compare
chrfwow
approved these changes
Feb 20, 2025
aepfli
force-pushed
the
fix/RPC_mode_not_using_deadline
branch
from
416060f to
d0fa1ad
Compare
During the refactoring of the connection the timeout for unary calls got lost. This adds the timeout back into the mix. TODO: - [ ] adapt gherkin tests for contextEnrichment to do handle stale seperately for file and inprocess Signed-off-by: Simon Schrottner <[email protected]>
aepfli
force-pushed
the
fix/RPC_mode_not_using_deadline
branch
from
d0fa1ad to
d898ace
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the refactoring of the connection the timeout for unary calls got lost. This adds the timeout back into the mix.
TODO:
How to test:
currently one e2e test is failing, which should fail, as this one should actually not work for rpc.
blocked by open-feature/flagd-testbed#229