[clientpython] feat(pyoaev): add multi-tenancy (#205)#208
[clientpython] feat(pyoaev): add multi-tenancy (#205)#208Megafredo wants to merge 2 commits intorelease/currentfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/current #208 +/- ##
==================================================
Coverage ? 70.63%
==================================================
Files ? 58
Lines ? 2350
Branches ? 0
==================================================
Hits ? 1660
Misses ? 690
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
15bc188 to
9e3ed3f
Compare
Kakudou
left a comment
There was a problem hiding this comment.
LGTM, everything seem in place, thanks. 🔥
If you can add some tests for that it would be perfects. 🤘
Kakudou
left a comment
There was a problem hiding this comment.
All good for me, every tests passes, and i'm able to see the tenant_id from the injector down to the client-python !
I've still have 3comments on the tests (nothing requiring a 'request changes', more a QoL):
- we could add a constraint to check that the uuid is in the format of an uuid4 (to be sure we don't get the default example ChangeMe for example)
- One assert is always True, so not really interesting
- we could create
_given, _when, _thenfunction from the gherkin features to even more emphasis and mutualized those steps if possible (like a common/conftest '_given_dummy_daemon(config)' )
There was a problem hiding this comment.
Maybe we could add a constraint 'tenant_id is not a valid uuid4' ?
| daemon = DummyDaemon(configuration=config) | ||
| assert daemon.api is not None | ||
|
|
||
| assert "openaev_tenant_id" in config_map or expected_tenant is None |
There was a problem hiding this comment.
Look like, it's an always assert True
If the tenant_id is missing -> True
If the tenant_id is valid -> True
So it doesn't really test 'if the tenant is propagated'.
Proposed changes
1. Client-side implementation using Python (
pyoaev)1.1 Managing the
tenant_idin the configuration (settings_loader.py)tenant_idfield toConfigLoaderOAEVtenant_id: UUIDtenant_id: UUID | Nonedefault=None,for now and replace it withdefault="2cffad3a-0001-4078-b0e2-ef74274022c3",once the migration is officially live1.2 Managing the
tenant_idin the pyoaev (client.py)tenant_id: Optional[UUID]to the client constructor_build_url()to support:base_urlandpathnormalizationurl_legacyif no tenant_id existsurl_tenantif tenant_id exists1.3 Managing the
tenant_idin the pyoaev (helper.py)OpenAEVInjectorHelpertenant_idfrom the config and sendOpenAEV1.4 Managing the
tenant_idin the pyoaev (utils.py)tenant_idtriggered duringPingAlivetenant_idwhen stoppingPingAlive1.5 Managing the
tenant_idin daemons (base_daemon.py)__get_default_api_clientwith the tenant_id2. Tests
3. Functional propagation tests from configurations
Related issues
Checklist
Further comments