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

Support config write-back #8

Open
flash1293 opened this issue Feb 5, 2024 · 8 comments
Open

Support config write-back #8

flash1293 opened this issue Feb 5, 2024 · 8 comments

Comments

@flash1293
Copy link
Contributor

flash1293 commented Feb 5, 2024

Via an Airbyte control message, the running connector can issue an update of its config object: https://docs.airbyte.com/understanding-airbyte/airbyte-protocol#airbytecontrolmessage

This is important for cases like single-use authentication tokens - some APIs only accept an authentication token once and return a new token in the response which has to be used the next time, invalidating the old token.

This message type is currently not honored by airbyte-lib - the message is silently dropped. Ideally, it's possible to handle this situation gracefully:

  • Automatically patch the config dictionary airbyte-lib works with
  • Signal to the user the config got changed so they can propagate the change to the appropriate place (as part of the sync result instance)
  • For airbyte-lib-validate-source: Update the config json file with the updated config

The last step is important to make it possible to run a proper check and read command as part of the CI steps - currently it's not possible to do this as integration test secrets are at risk of being invalidated by the test being run: airbytehq/airbyte#34044

@flash1293
Copy link
Contributor Author

@aaronsteers we talked about this before - there's another situation where the config update needs to be handled and that's the CI check that's run for airbyte-lib enabled connectors.

@aaronsteers aaronsteers transferred this issue from airbytehq/airbyte Feb 6, 2024
@aaronsteers aaronsteers changed the title airbyte-lib: Support config update Support config update Feb 6, 2024
@aaronsteers
Copy link
Contributor

Made a request in slack to get more detail on scope here and which connectors are affected:

To help us prioritize when/if to build support for the CONTROL message in AirbytLib as documented here, I'm looking for a list of python source connectors which require what I'll call "config write-back". My understanding is that this is primarily for replacing retired refresh_token secret values. Does anyone know how to locate those connectors that require this feature, and/or know of a few which I can use to better analyze/understand the technical requirements?

@aaronsteers
Copy link
Contributor

aaronsteers commented Feb 6, 2024

Aggregating responses from Slack thread, it looks like we have at least these that require the feature:

  • source-airtable
  • source-trustpilot
  • source-xero
  • source-gitlab
  • source-typeform
  • source-quickbooks (updated per comment below)

And a way to search for them would be to look for instances of refresh_token_updater and/or SingleUseRefreshTokenOauth2Authenticator.

Thanks to @flash1293, @pedroslopez, and @alafanechere for their expertise here. 🙏

@aaronsteers aaronsteers changed the title Support config update Support config write-back Feb 8, 2024
@andreibaragan
Copy link

source-quickbooks is another. According to Quickbooks docs, they update the value of the refresh_token value every 24 hours, or the next time you refresh the access tokens after 24 hours.

@aaronsteers
Copy link
Contributor

aaronsteers commented Apr 16, 2024

@andreibaragan - Thanks for raising. I've updated my comment above to include quickbooks as well.

For our information and prioritization, are you blocked by this or do you have a workaround?

@andreibaragan
Copy link

andreibaragan commented Apr 16, 2024

@aaronsteers this is blocking.

@andreibaragan
Copy link

@aaronsteers is there any update/plan/alternative to this issue?

@aaronsteers
Copy link
Contributor

aaronsteers commented Aug 8, 2024

@andreibaragan - We have not made any progress on this and it is not currently prioritized. What we can do in the meanwhile is to try to work out a spec, which then Airbyte or a member of the community could pick up.

To keep this conversation moving, let me put some implementation details here in this thread, according to my research and current understanding of the feature requirements.

For context and background: Unlike Airbyte Platform, PyAirbyte has no write access to your secret store - nor do we really want PyAirbyte to need this access. PyAirbyte also does not (yet) have a file-based interface for reading config information. Meaning, there's no way for PyAirbyte to handle the new creds that it would receive from the AirbyteControlMessage.

A few options:

  1. Add Callback.
    • The get_source() method could accept something like a control_message_callback or (more specifically) config_change_callback which would be called by PyAirbyte whenever a control message is received from the source, aka when the config is being attempted to be changed.
    • The user would have full control to handle this config change when/if a message is received.
  2. Add file-based config option.
    • The get_source() could accept a config file where it currently expects a config dictionary.
    • PyAirbyte would write config changes to the file when/if the control message from the connector instructs it to.
    • PyAirbyte would print to the logs, but otherwise the process would be invisible to the user.
  3. No change to function signatures, except treat config as okay-to-modify.
    • Same as option 2, but we write config changes back to the dictionary.
    • Similarly, we print to the logs, but otherwise the process is invisible to the user.

With options 2 or 3, we don't change much, but we also don't have any explicit handoff that the config change is received by the user or handled appropriate. If the process crashes, the python dictionary's contents are lost. And in ephemeral environments, the local config file is likewise very likely to be dropped at the end of the sync.

Given the above prelim spec exploration, I think I slightly prefer using option 1, which would be to add an option for an explicit callback.

Lmk if this makes sense to you, or if you have any other ideas or suggestions based on your use case.

Once we have a viable spec, I can at least mark this as ready to work, which is a step in the right direction, I think.

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

No branches or pull requests

3 participants