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: Improve wait logic to a more elegant solution #1160 #1169

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

chrfwow
Copy link
Contributor

@chrfwow chrfwow commented Jan 21, 2025

This PR

Adds a better wait logic to wait for the initialization of the FlagdProvider.

With these changes, it is not advisable to restart a FlagdProvider once it has been shut down.

Related Issues

Fixes #1160

…ant-solution-open-feature#1160

# Conflicts:
#	providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java
#	providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java
#	providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java
* for sync blocks.
*/
@Getter
public class EventsLock {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this class out of the FlagdProvider to ensure no private fields are accessed and to keep the size of the file down.
However, I feel like the name is not approrpaite anymore, so please feel free to suggest a better one. Also, this new class is strongly coupled to the FlagdProvider, if you have any suggestions to make it cleaner please let me know

Copy link
Member

Choose a reason for hiding this comment

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

maybe FlagdProviderSyncState or similar could be an appropriate name - or just SyncState or SyncResource - but to be fair, i am also not hundred percent happy with my suggestions

@chrfwow chrfwow marked this pull request as ready for review January 21, 2025 10:24
@chrfwow chrfwow requested a review from a team as a code owner January 21, 2025 10:24
Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

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

thank you - i just added some questions and suggestions, nothing crucial except the one about where we are doing this wait and notify

* for sync blocks.
*/
@Getter
public class EventsLock {
Copy link
Member

Choose a reason for hiding this comment

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

maybe FlagdProviderSyncState or similar could be an appropriate name - or just SyncState or SyncResource - but to be fair, i am also not hundred percent happy with my suggestions

@chrfwow chrfwow marked this pull request as draft January 22, 2025 08:12
@chrfwow
Copy link
Contributor Author

chrfwow commented Jan 29, 2025

Waiting on open-feature/java-sdk#1299 completed

…open-feature#1160

# Conflicts:
#	providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java
Signed-off-by: christian.lutnik <[email protected]>
Signed-off-by: christian.lutnik <[email protected]>
Signed-off-by: christian.lutnik <[email protected]>
Signed-off-by: christian.lutnik <[email protected]>
Signed-off-by: christian.lutnik <[email protected]>
@chrfwow chrfwow marked this pull request as ready for review February 18, 2025 13:20
Signed-off-by: christian.lutnik <[email protected]>
…open-feature#1160

# Conflicts:
#	providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java
#	providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java
Signed-off-by: christian.lutnik <[email protected]>
@chrfwow chrfwow requested review from gruebel and liran2000 February 25, 2025 07:24
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.

[flagd] improve wait logic to a more elegant solution
6 participants