feat(Dogfooding): Add OpenFeature SDK for server-side Flagsmith-on-Flagsmith#7008
feat(Dogfooding): Add OpenFeature SDK for server-side Flagsmith-on-Flagsmith#7008
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.
Once credits are available, reopen this pull request to trigger a review.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
github-actions
bot
added
api
Docker builds report
|
| variants={"enabled": True}, | ||
| default_variant="enabled", |
There was a problem hiding this comment.
Yes, this is how to create an enabled by default in-memory flag for OF. No, there is no shorter way to do this.
There was a problem hiding this comment.
NIT: I got a bit confused it because of enabled. Bit of a detail just for next readings, do you think we could rename it enabled_variant to avoid thinking it's related to the enabled property ?
There was a problem hiding this comment.
TBH I'm not sure if it's possible to make this code less confusing, I'd just let it be for now.
| [[tool.mypy.overrides]] | ||
| module = ["openfeature_flagsmith.*"] | ||
| ignore_missing_imports = true |
There was a problem hiding this comment.
Opened Flagsmith/flagsmith-openfeature-provider-python#30 in the meantime.
khvn26
changed the title
khvn26
changed the title
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7008 +/- ##
==========================================
- Coverage 98.33% 98.33% -0.01%
==========================================
Files 1335 1335
Lines 49943 49940 -3
==========================================
- Hits 49113 49110 -3
Misses 830 830 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Zaimwa9
left a comment
Zaimwa9
left a comment
There was a problem hiding this comment.
Not much too say, I would approve if it wasn't for the dependency update and merge conflicts to come. Good addition overall
| variants={"enabled": True}, | ||
| default_variant="enabled", |
There was a problem hiding this comment.
NIT: I got a bit confused it because of enabled. Bit of a detail just for next readings, do you think we could rename it enabled_variant to avoid thinking it's related to the enabled property ?
Migrate Flagsmith-on-Flagsmith feature evaluation from the native Flagsmith Python SDK to the vendor-neutral OpenFeature SDK with the Flagsmith provider, dogfooding our own OpenFeature integration.
khvn26
force-pushed
the
feat/openfeature-sdk-migration
branch
from
ddf8c99 to
0d6327b
Compare
github-actions
bot
added
feature
| ) -> OpenFeatureClient: | ||
| openfeature_client = openfeature_api.get_client(domain=domain) | ||
| if openfeature_client.get_provider_status() != ProviderStatus.READY: | ||
| initialise_provider(domain, **(flagsmith_kwargs or get_provider_kwargs())) |
There was a problem hiding this comment.
Are we sure about this code? It looks like a ticking bomb. If we pass in an argument, we don't set any other property, not even the api key?
The only other callsite is in this test that only asserts the given parameter (local evaluation)
2 participants
docs/if required so people know about the feature.Changes
Migrate Flagsmith-on-Flagsmith feature evaluation from the native Flagsmith Python SDK to the vendor-neutral OpenFeature SDK with the Flagsmith provider.
Added
openfeature-sdkandopenfeature-provider-flagsmith(>=0.1.6), now published on PyPI with flagsmith SDK v5 support.Replaced
get_client()withget_openfeature_client()inintegrations/flagsmith/client.py, using OpenFeature's provider registry with a named domain (flagsmith-api) and lazy initialisation viaProviderStatuscheck.Replaced
flagsmith_identifier+flagsmith_on_flagsmith_api_traitson the Organisation model with a singleopenfeature_evaluation_contextproperty returning anEvaluationContext.Migrated all call-sites from
client.get_identity_flags(...).is_feature_enabled("x")toclient.get_boolean_value("x", default_value=False, evaluation_context=...), including leftoverflags.is_feature_enabled()references inorganisations/tasks.pyandorganisations/task_helpers.py.enable_featurestest fixture now uses OpenFeature'sInMemoryProviderinstead of mocking Flagsmith internals. 22 org tasks tests migrated from manualget_openfeature_clientmocks to the fixture.Added
openfeature_flagsmith.*andlicensing.*to mypyignore_missing_importsoverrides (packages lackpy.typed).How did you test this code?
Made sure existing tests pass.