Skip to content

feat: add Unleash provider #424

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 20 commits into from
Sep 28, 2023
Merged

feat: add Unleash provider #424

merged 20 commits into from
Sep 28, 2023

Conversation

@liran2000
Copy link
Member

@liran2000 liran2000 commented Sep 7, 2023

add Unleash provider.

See Readme for details.

Discussed at Unleash.

@liran2000
Copy link
Member Author

liran2000 commented Sep 7, 2023 •
edited
Loading

hi @pransh15, @chriswk, @ivarconr, @gastonfournier,

you can have a look and share your thoughts.

Also to be reviewed from OpenFeature perspective.

gastonfournier
gastonfournier reviewed Sep 8, 2023
View reviewed changes
Copy link

@gastonfournier gastonfournier left a comment

Choose a reason for hiding this comment

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

This looks promising! I leave you a few comments to make it better, but I'm glad you're doing this!

It might be worthy adding a comment in https://github.com/orgs/Unleash/discussions/3896 to engage other contributors.

@toddbaert toddbaert self-requested a review September 8, 2023 14:39
@liran2000
Copy link
Member Author

liran2000 commented Sep 8, 2023 •
edited
Loading

This looks promising! I leave you a few comments to make it better, but I'm glad you're doing this!

It might be worthy adding a comment in https://github.com/orgs/Unleash/discussions/3896 to engage other contributors.

Thanks @gastonfournier !
I made some adjustments based on your comments.
Since you probably more familiar with Unleash, I am interested in opinions on the concepts of this provider as described at the Readme:

Concepts

  • Boolean evaluation gets feature enabled status.
  • String evaluation gets feature variant value.

@liran2000
- some fixes
5eef0e2 
- test using WireMock

Signed-off-by: liran2000 <[email protected]>
@liran2000
update WireMock version to match SDK Java version
f67742a 
Signed-off-by: liran2000 <[email protected]>
@liran2000
add shutdown method
92e3a62 
Signed-off-by: liran2000 <[email protected]>
@liran2000
tests updates
8a63167 
Signed-off-by: liran2000 <[email protected]>
ivarconr
ivarconr reviewed Sep 11, 2023
View reviewed changes
@liran2000 liran2000 mentioned this pull request Sep 11, 2023
Closed
@liran2000
- focused test features
99dd6a7 
- update name
- object evaluation

Signed-off-by: liran2000 <[email protected]>
@liran2000
enhancements
8f93bbb 
- int,double evaluation
- flag metadata

Signed-off-by: liran2000 <[email protected]>
@liran2000
enhance test
1ddfd61 
Signed-off-by: liran2000 <[email protected]>
@liran2000
Copy link
Member Author

liran2000 commented Sep 18, 2023

hi @ivarconr what do you think about proceeding with this PR and adding Unleash provider here ?
basically most of the comments are addressed.

@liran2000 liran2000 marked this pull request as ready for review September 20, 2023 16:59
@liran2000 liran2000 requested a review from a team as a code owner September 20, 2023 16:59
@ivarconr
Copy link
Contributor

ivarconr commented Sep 25, 2023

hi @ivarconr what do you think about proceeding with this PR and adding Unleash provider here ?
basically most of the comments are addressed.

All in favor. Would love to get to test it in an example app, either as a alpha release or if you provide snapshot versions.

ivarconr
ivarconr reviewed Sep 27, 2023
View reviewed changes
@toddbaert @ivarconr
Update providers/unleash/README.md
c0ddb56 
Co-authored-by: Ivar Conradi Østhus <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
toddbaert
toddbaert reviewed Sep 27, 2023
View reviewed changes
@toddbaert
Copy link
Member

toddbaert commented Sep 27, 2023 •
edited
Loading

@liran2000 the release setup looks good. When we merge this and the associated release PR, it will release a 0.0.2 version which is fine with me. We don't publish snapshots (in the java sense) from this repo, but we could (I believe) make the version number something like 0.0.1-alpha or 0.0.1-prerelease if you'd like to have something that seems less "final" for now, which might be a good idea. cc @ivarconr

@liran2000 Thanks for all your efforts on this. It's been requested by the community more than once!

Please see the 2 unresolved issues above.

@liran2000
update owners and test
4a61e9c 
Signed-off-by: liran2000 <[email protected]>
@github-actions github-actions bot requested a review from sighphyre September 27, 2023 15:05
@liran2000
Copy link
Member Author

liran2000 commented Sep 27, 2023

Thanks :)
addressed latest comments.

@liran2000
update version to alpha
e50eaff 
Signed-off-by: liran2000 <[email protected]>
toddbaert
toddbaert approved these changes Sep 27, 2023
View reviewed changes
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

I'll merge this before EOD unless I hear objections.

Thanks again all, especially @liran2000

@toddbaert toddbaert merged commit ec6b3c9 into open-feature:main Sep 28, 2023
@github-actions github-actions bot mentioned this pull request Sep 28, 2023
Merged
@liran2000 liran2000 deleted the feature/unleash branch September 28, 2023 05:43
@liran2000 liran2000 mentioned this pull request Oct 29, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@toddbaert toddbaert toddbaert approved these changes

@sighphyre sighphyre Awaiting requested review from sighphyre

+2 more reviewers

@ivarconr ivarconr ivarconr left review comments

@gastonfournier gastonfournier gastonfournier left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@liran2000 liran2000

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@liran2000 @ivarconr @toddbaert @gastonfournier @sighphyre