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.

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

refactor!: Add EventHandler + Combine #29

Merged
merged 15 commits into from
Jan 24, 2024
Merged

refactor!: Add EventHandler + Combine #29

merged 15 commits into from
Jan 24, 2024

Conversation

fabriziodemaria
Copy link
Contributor

@fabriziodemaria fabriziodemaria commented Jan 18, 2024 •
edited
Loading

This PR

  • Removes event APIs at the client level (will be re-introduced when multiple clients/providers are supported)
  • Adds an EventHandler implementation that Providers can use to emit events (a FeatureProvider now extends the EventPublisher protocol)
  • Adds Combine as the API for eventing

Notes

Following patterns implemented in the Kotlin SDK as much as possible, for consistency

Follow-up Tasks

  • Add setProviderAndWait function in OpenFeatureAPI
  • Events should contain more metadata than just their type
  • Introduce ProviderStatus as an entity separated from ProviderEvent (also for Kotlin)

@fabriziodemaria fabriziodemaria force-pushed the event-handler branch 2 times, most recently from 57375d6 to 3543cd2 Compare January 18, 2024 15:16
@nicklasl nicklasl changed the title refactor: Add EventHandler refactor!: Add EventHandler Jan 19, 2024
nicklasl
nicklasl reviewed Jan 19, 2024
View reviewed changes
Copy link
Member

@nicklasl nicklasl left a comment

Choose a reason for hiding this comment

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

Added some comments on the tests part.

To make sure we test all different event cases on the OpenFeatureAPI.shared level, we can use a Provider which have a public accessible EventHandler (so we can remotely control it to emit events).

@fabriziodemaria fabriziodemaria force-pushed the event-handler branch 2 times, most recently from 94a1a74 to 266b5e6 Compare January 19, 2024 10:11
@fabriziodemaria fabriziodemaria force-pushed the event-handler branch 2 times, most recently from d4103c8 to 16a0e8b Compare January 19, 2024 14:03
@fabriziodemaria fabriziodemaria marked this pull request as draft January 19, 2024 14:04
@fabriziodemaria fabriziodemaria marked this pull request as ready for review January 22, 2024 13:40
@fabriziodemaria fabriziodemaria force-pushed the event-handler branch 2 times, most recently from e3029f7 to 9ca2cc3 Compare January 22, 2024 13:51
@fabriziodemaria fabriziodemaria changed the title refactor!: Add EventHandler refactor!: Add EventHandler + Combine Jan 22, 2024
@vahidlazio vahidlazio force-pushed the event-handler branch 4 times, most recently from b744531 to d903315 Compare January 23, 2024 15:12
@vahidlazio vahidlazio force-pushed the event-handler branch 6 times, most recently from 61ddf6f to 1efff09 Compare January 23, 2024 17:01
@fabriziodemaria
refactor: Add EventHandler
df30a35 
Signed-off-by: Fabrizio Demaria <[email protected]>
fabriziodemaria and others added 13 commits January 24, 2024 10:49
@fabriziodemaria
[WIP] Make OpenFeatureAPI implement EventPublisher
058911e 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
[WIP] Introduce Combine for Events
f483c6f 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
Use Subject instead of NotificationCenter
19eedee 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
Rename eventing protocols
70e3237 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
Client-side ctx change eventing fix
bc0bae7 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
Rename subject variable
12284d0 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
Add global events
7053c0b 
Signed-off-by: Fabrizio Demaria <[email protected]>
@fabriziodemaria
Improve testing on global events
f29c1b5 
Signed-off-by: Fabrizio Demaria <[email protected]>
@vahidlazio @fabriziodemaria
add provider subject and listen to the changes coming from the provider
4f67db4 
Signed-off-by: vahid torkaman <[email protected]>
@vahidlazio @fabriziodemaria
use swithToLatest and drop listening for the provider inside the Open…
4d36a25 
…FeatureAPI

Signed-off-by: vahid torkaman <[email protected]>
@vahidlazio @fabriziodemaria
use compactMap to drop the nil values
edacbcc 
Signed-off-by: vahid torkaman <[email protected]>
@vahidlazio @fabriziodemaria
map to empty stream in case of setting provider to nil
9c128cc 
Signed-off-by: vahid torkaman <[email protected]>
@fabriziodemaria
Minor refactoring
2ae4cb9 
Signed-off-by: Fabrizio Demaria <[email protected]>
nicklasl
nicklasl approved these changes Jan 24, 2024
View reviewed changes
@fabriziodemaria
Remove unused code
2ba03fe 
Signed-off-by: Fabrizio Demaria <[email protected]>
@nicklasl nicklasl mentioned this pull request Jan 24, 2024
Open
@nicklasl nicklasl merged commit dd122f7 into main Jan 24, 2024
5 checks passed
@github-actions github-actions bot mentioned this pull request Jan 22, 2024
Merged
@fabriziodemaria fabriziodemaria deleted the event-handler branch January 24, 2024 15:22
fabriziodemaria pushed a commit that referenced this pull request Feb 2, 2024
@github-actions
chore(main): release 0.1.0 (#20)
02b033c 
🤖 I have created a release *beep* *boop*
---


##
[0.1.0](0.0.2...0.1.0)
(2024-02-02)


### ⚠ BREAKING CHANGES

* Add EventHandler + Combine
([#29](#29))

### 🐛 Bug Fixes

* setProviderAndWait does not hang on ProviderError
([#35](#35))
([5661080](5661080))


### ✨ New Features

* Add ProviderNotReady event
([#36](#36))
([389f117](389f117))
* Add setProviderAndWait
([#30](#30))
([3ce6b8d](3ce6b8d))


### 🧹 Chore

* **deps:** update actions/checkout action to v4
([#18](#18))
([eb0cd56](eb0cd56))
* **deps:** update dependency apple/swift-format to v509
([#21](#21))
([5f12304](5f12304))
* Smaller cleanup in tests
([#33](#33))
([053dabc](053dabc))


### 📚 Documentation

* add sections for logging, named providers, and shutdown
([#31](#31))
([dc5876c](dc5876c))
* Fix Installation documentation
([#27](#27))
([5ddf45d](5ddf45d)),
closes [#25](#25)
* Update README to latest template
([#28](#28))
([dbdd502](dbdd502))
* Update README.md
([#19](#19))
([ec599ff](ec599ff))


### 🔄 Refactoring

* Add EventHandler + Combine
([#29](#29))
([dd122f7](dd122f7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreas-karlsson andreas-karlsson andreas-karlsson left review comments

@beeme1mr beeme1mr beeme1mr left review comments

@nicklasl nicklasl nicklasl approved these changes

@vahidlazio vahidlazio Awaiting requested review from vahidlazio

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fabriziodemaria @andreas-karlsson @beeme1mr @vahidlazio @nicklasl