Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: attempts with connection improvements #118

Conversation

aepfli
Copy link
Member

@aepfli aepfli commented Dec 11, 2024

Based on open-feature/flagd#1472 we're improving our reconnection logic.

We're using GRPC, and GRPC has already offers a reconnection with a backoff.
Within this PR:

  • we're moving to the reconnect mechanism of GRPC
  • we're utilizing Wait-for-ready to wait for our stream to have a connection again
  • we're migrating to a timer-based error approach (as we can't distinguish the number of connection attempts anymore)
  • Adding a shutdown for the loop in the filewatcher

@aepfli aepfli force-pushed the feat/grace_attempts_with_connection_improvements branch 2 times, most recently from fa3ef6c to ae80e4b Compare December 11, 2024 14:41
Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 95.83333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.00%. Comparing base (f156ea5) to head (fe1df5d).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...enfeature/contrib/provider/flagd/resolvers/grpc.py 95.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #118      +/-   ##
==========================================
- Coverage   94.25%   94.00%   -0.25%     
==========================================
  Files          14       14              
  Lines         731      734       +3     
==========================================
+ Hits          689      690       +1     
- Misses         42       44       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aepfli aepfli force-pushed the feat/grace_attempts_with_connection_improvements branch 11 times, most recently from 76004bc to abdb4c9 Compare December 18, 2024 06:49
@aepfli aepfli marked this pull request as ready for review December 18, 2024 06:50
@aepfli aepfli requested a review from a team as a code owner December 18, 2024 06:50
@aepfli
Copy link
Member Author

aepfli commented Dec 18, 2024

Copy link
Member

@gruebel gruebel left a comment

Choose a reason for hiding this comment

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

@aepfli let me know, when the blocker is resolved and it is good to merge

@aepfli aepfli force-pushed the feat/grace_attempts_with_connection_improvements branch from abdb4c9 to 86dd9fd Compare December 25, 2024 20:41
Signed-off-by: Simon Schrottner <[email protected]>
@aepfli aepfli force-pushed the feat/grace_attempts_with_connection_improvements branch from 5140a2c to fe1df5d Compare December 27, 2024 10:42
@gruebel
Copy link
Member

gruebel commented Dec 27, 2024

@aepfli just to make sure, is it ready to merge now? 😅

@aepfli
Copy link
Member Author

aepfli commented Dec 27, 2024

@aepfli just to make sure, is it ready to merge now? 😅

Yes I think it is ready. The tests are also more stable ;)

@gruebel gruebel merged commit 8e23a70 into open-feature:main Dec 27, 2024
25 checks passed
@aepfli aepfli deleted the feat/grace_attempts_with_connection_improvements branch December 27, 2024 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants