Repository navigation
fix(low-code): build session authenticators and paginators nested in SelectiveAuthenticator or custom components - #500
Conversation
📝 WalkthroughWalkthroughThe change updates the method signature of Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ModelToComponentFactory
Caller->>ModelToComponentFactory: create_session_token_authenticator(model, config, name=...)
ModelToComponentFactory-->>Caller: Returns ApiKeyAuthenticator or BearerAuthenticator
Would you like to update any related documentation or usage examples to reflect the new keyword-only requirement for the Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Hi Airbyte team Brian Lai (@brianjlai) Aldo Gonzalez (@aldogonzalez8), Could you help with reviewing this PR please? I'm not sure how to config the |
…custom component Change `name` argument in the function `create_session_token_authenticator` to a keyword-only argument so that it can be injected via `$parameters`. Other `name` arguments in the same module are already keyword-only.
Baz (bazarnov)
left a comment
There was a problem hiding this comment.
Thanks hai-ld for the original fix, and sorry it waited this long.
I extended it to close #422 as well:
- merged
main; create_selective_authenticatornow forwards the requester kwargs to its nested authenticators, soSessionTokenAuthenticator(name) andLegacySessionTokenAuthenticator(url_base) build inside aSelectiveAuthenticator, with the stream name, the same as on anHttpRequester;- factory tests for both halves plus one end-to-end read through
ConcurrentDeclarativeSource; every new case fails onmain, and each production line is pinned by a mutation.
No monorepo manifest changes behavior (0 of 29 SelectiveAuthenticator users nest a session authenticator). The remaining nesting gaps under custom components are tracked in #1186.
The connector checks on this PR fail only because GSM secrets are not available to fork PRs (No Google Cloud credentials found in secrets fetch); the same commit passed all of them in a base-repo run: https://github.com/airbytehq/airbyte-python-cdk/actions/runs/36428824962. destination-motherduck is red on main as well.
…nators under custom components
This builds on hai-ld's original change in this PR:
namebecomes keyword-only increate_session_token_authenticator, so aSessionTokenAuthenticatorcan be created as a subcomponent of a custom component. I mergedmain, added theSelectiveAuthenticatorhalf of #422, the remaining nested-under-custom-component cases from #1186, and tests for all of them.Summary
Three related defects in how the factory passes args to nested components. A and B end in the same
TypeErrorfromcreate_session_token_authenticator:SelectiveAuthenticatordrops the requester kwargs (the [SelectiveAuthenticator] TypeError when attempting to create SessionTokenAuthenticator #422 body).create_selective_authenticatorbuilt every nested authenticator without kwargs. It forwarded**kwargsonly intoSelectiveAuthenticator(...), whose__new__discards them and returns the selected child. So thename,url_baseanddecoderthatcreate_http_requesterpasses never reached the nested authenticators.SessionTokenAuthenticator(requiresname) andLegacySessionTokenAuthenticator(requiresurl_base) failed at stream build time incheck,discoverandread. All branches are built eagerly, so the error also happened when the session branch was not selected.nameis filtered out under custom components ([SelectiveAuthenticator] TypeError when attempting to create SessionTokenAuthenticator #422 comment 1, and the original case of this PR)._create_nested_componentforwards only keyword-only constructor args, from the parent's kwargs or from$parameters.namewas positional-or-keyword, so it was always dropped, including the documented$parameters.nameescape hatch.create_selective_authenticatorhad no keyword-only args, so aSelectiveAuthenticatordirectly under a custom component (e.g.CustomRequester.authenticator) got nothing to pass to a session branch, and the hint it raised (Please provide SelectiveAuthenticator.$parameters.name) could not be followed.CursorPaginationandOffsetIncrementrequire a positionaldecoder, which a custom parent cannot pass and$parameterscannot hold.Changes
create_selective_authenticatorforwards**kwargsto each nested_create_component_from_model. The comprehension variable is renamedname>>key, so the selector key is not confused with the stream name. The**kwargspassed toSelectiveAuthenticator(...)stays as it was.create_session_token_authenticatortakesnameas keyword-only (hai-ld's change). When the parent passesname, as aCustomAuthenticatoron anHttpRequesterdoes, the parent's stream name wins over$parameters.name(model_to_component_factory.py:2057).$parameters.nameapplies only when no parent passesname.create_selective_authenticatortakesnameandurl_baseas keyword-only args and forwards the non-None ones. Under a custom component they come from the parent's kwargs or fromSelectiveAuthenticator.$parameters, so the existing hint now works; a missing value still raises it.decoderis not added: no built-in authenticator reads it.create_cursor_paginationandcreate_offset_incrementdefaultdecodertoJsonDecoder, the fallbackcreate_default_paginatoralready uses. Explicit decoders are unchanged.Why the stream name and not the selector key
#422 comment 2 suggests passing the selector key as
name. I forwarded the requester kwargs instead, so a nested authenticator gets exactly what it gets when set directly on anHttpRequester:<stream>_login_requester, as it is withoutSelectiveAuthenticator. That name sets theairbyte.HttpRequester.<name>logger (requesters/http_requester.py:272) and theStreamDescriptoron traced errors (streams/http/http_client.py:355,363) and on the rate-limited stream status (http_client.py:588). The selector key would name every stream's login requestersession_login_requester.nameis the context in the REDUCE_PAGE_SIZE rejection message for the login requester (model_to_component_factory.py:1215-1217).LegacySessionTokenAuthenticatorneedsurl_base(model_to_component_factory.py:3475-3477), which a selector key cannot supply.Compatibility
$refs resolved: 29 connectors useSelectiveAuthenticator, and none nestsSessionTokenAuthenticatororLegacySessionTokenAuthenticator(branches: OAuth 23, Bearer 16, Basic 7, ApiKey 5, Custom 3). Every factory in theSelectiveAuthenticator.authenticatorsunion accepts**kwargs. Only the two session factories usenameorurl_base, so the other branches build as before.CustomAuthenticatorinside aSelectiveAuthenticatornow receivesname,url_baseanddecoderand passes them to its nested components, the same as aCustomAuthenticatorset directly on anHttpRequester.create_custom_componentlets these override same-named manifest fields (model_to_component_factory.py:1887-1890), and a nested requester's$parameters.nameis replaced by the stream name, as it already is when theCustomAuthenticatorsits on theHttpRequesterdirectly. The 3 connectors with this shape (source-jira, source-okta, source-salesloft) declare none of these fields and have no nested components in them.SelectiveAuthenticatordirectly under a custom component: 5 connectors (source-asana, source-hubspot, source-linkedin-ads, source-mixpanel, source-slack), all with OAuth, Bearer, ApiKey or Basic branches, whose factories ignorenameandurl_base. No monorepoCursorPaginationorOffsetIncrementsits directly under a custom component.namekeyword-only only affects positional callers ofcreate_session_token_authenticator. There are none: the CDK calls it through the constructor map withname=, and no monorepo connector calls it.Tests
parsers/test_model_to_component_factory.py, next totest_create_requester_with_selective_authenticator:test_create_requester_with_selective_authenticator_nesting_session_authenticators[session|legacy|token|oauth]: oneSelectiveAuthenticatorwith all four branches. Each selection builds, which also covers the eager build of unselected session branches. Thelegacycase also checks that the requester'surl_basereachesLegacySessionTokenAuthenticator.test_create_requester_with_selective_authenticator_passes_stream_name_to_session_token_authenticator:login_requester.name == "lists_login_requester".test_create_requester_with_custom_authenticator_nesting_session_token_authenticator: the parent'snamereaches the nestedSessionTokenAuthenticator.test_create_custom_authenticator_nesting_session_token_authenticator_with_name_in_parameters:$parameters.nameworks when the parent passes noname.test_create_requester_with_selective_authenticator_nesting_custom_authenticator:SelectiveAuthenticator>>CustomAuthenticator>>SessionTokenAuthenticator, which needs both changes.test_concurrent_declarative_source.py::test_given_selective_authenticator_nesting_session_token_authenticator_when_read_then_authenticate: end to end throughConcurrentDeclarativeSourceandHttpMocker. Withsessionselected there is onePOST /loginand the session header on the data request. Withtokenselected, records are returned and no login request is sent.parsers/testing_components.py:TestingCustomAuthenticator, a custom authenticator that wraps a nested one.parsers/test_model_to_component_factory.py:test_create_custom_requester_with_selective_authenticator_*(session branch gets the stream name; legacy branch gets$parameters.url_base; without it, the hint is raised) andtest_parse_custom_component_fields_if_subcomponent[test_cursor_pagination_without_decoder|test_offset_increment_without_decoder]. 4 fail before this change; the hint case is a guard.All 10 #422 cases fail on
main. The existingtest_create_requester_with_selective_authenticatorcases still pass.Mutation check, each production change reverted or altered alone:
create_selective_authenticatorSelectiveAuthenticatorcase, both end-to-end cases)namepositional againCustomAuthenticatorcase)nameSelectiveAuthenticator)nameforwarded, nourl_basenesting_session_authenticatorscase)url_baseforwardedlegacycase)unit_tests/sources/declarative/parsers,authandrequesters/paginators: 715 passed.ruff format,ruff checkandmypyon the factory: clean.Fixes #422
Fixes #1186