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

fix: only shutdown providers that are not attached to any client #444

Merged

Conversation

lukas-reining
Copy link
Member

This PR

Providers that are bound to several named clients are closed if another provider is set for one of these clients.
This PR only closes providers if they are not used by any client.

Related Issues

Fixes #443

Notes

Follow-up Tasks

How to test

Added tests for server and client.

Signed-off-by: Lukas Reining <lukas.reining@codecentric.de>
@codecov
Copy link

codecov bot commented May 31, 2023

Codecov Report

Merging #444 (05cca27) into main (5d55ea1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 05cca27 differs from pull request most recent head 8a3873f. Consider uploading reports for the commit 8a3873f to get more accurate results

@@           Coverage Diff           @@
##             main     #444   +/-   ##
=======================================
  Coverage   99.47%   99.47%           
=======================================
  Files          25       25           
  Lines        2468     2469    +1     
  Branches      236      236           
=======================================
+ Hits         2455     2456    +1     
  Misses         13       13           
Impacted Files Coverage Δ
packages/shared/src/open-feature.ts 99.14% <100.00%> (+<0.01%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Nice change

packages/shared/src/open-feature.ts Outdated Show resolved Hide resolved
@beeme1mr beeme1mr requested review from weyert and toddbaert May 31, 2023 17:26
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Signed-off-by: Lukas Reining <lukas.reining@codecentric.de>
@beeme1mr
Copy link
Member

beeme1mr commented Jun 1, 2023

@toddbaert are you waiting for the spec change to be merged? If so, could you link that PR here?

@toddbaert
Copy link
Member

@toddbaert are you waiting for the spec change to be merged? If so, could you link that PR here?

Ya, I'm waiting for open-feature/spec#193 just to avoid any thrashing.

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.

This LGTM. I will wait to merge once open-feature/spec#193 is merged.

@toddbaert toddbaert merged commit 7e469c4 into open-feature:main Jun 2, 2023
beeme1mr pushed a commit that referenced this pull request Jun 6, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.0.4](shared-v0.0.3...shared-v0.0.4)
(2023-06-06)


### Features

* add init/shutdown and events
([#436](#436))
([5d55ea1](5d55ea1))
* add named client support
([#429](#429))
([310c6ac](310c6ac))
* add support for flag metadata
([#426](#426))
([029ec26](029ec26))


### Bug Fixes

* only shutdown providers that are not attached to any client
([#444](#444))
([7e469c4](7e469c4))

---
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>
beeme1mr pushed a commit that referenced this pull request Jun 6, 2023
🤖 I have created a release *beep* *boop*
---


##
[0.3.3-experimental](web-sdk-v0.3.2-experimental...web-sdk-v0.3.3-experimental)
(2023-06-06)


### Features

* add init/shutdown and events
([#436](#436))
([5d55ea1](5d55ea1))
* add named client support
([#429](#429))
([310c6ac](310c6ac))
* add support for flag metadata
([#426](#426))
([029ec26](029ec26))


### Bug Fixes

* bundlers wrongly resolving server/client modules
([#445](#445))
([6acddd5](6acddd5))
* only shutdown providers that are not attached to any client
([#444](#444))
([7e469c4](7e469c4))

---
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>
beeme1mr added a commit that referenced this pull request Jun 6, 2023
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](js-sdk-v1.2.0...js-sdk-v1.3.0)
(2023-06-06)


### Features

* add init/shutdown and events
([#436](#436))
([5d55ea1](5d55ea1))
* add named client support
([#429](#429))
([310c6ac](310c6ac))
* add support for flag metadata
([#426](#426))
([029ec26](029ec26))


### Bug Fixes

* bundlers wrongly resolving server/client modules
([#445](#445))
([6acddd5](6acddd5))
* only shutdown providers that are not attached to any client
([#444](#444))
([7e469c4](7e469c4))

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

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
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.

[BUG] Bound providers are shutdown while attached to clients
3 participants