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

Update configuration from control messages in synchronous commands #20145

Closed
pedroslopez opened this issue Dec 6, 2022 · 3 comments · Fixed by #20894
Closed

Update configuration from control messages in synchronous commands #20145

pedroslopez opened this issue Dec 6, 2022 · 3 comments · Fixed by #20894
Assignees

Comments

@pedroslopez
Copy link
Contributor

pedroslopez commented Dec 6, 2022

Following #17909, which introduced the capability for the platform to update connector configuration during syncs, we also want to update configuration during the other commands (e.g. check, discover)

We can use the UpdateConnectorConfigHelper introduced in the previous PR to perform the updating, which already handles things like masking secrets.

This will involve initializing the AirbyteApiClient in airbyte-server, as well as updating inputs to the jobs as required to have the right information for calling the endpoints (e.g. source or destination id)

@pedroslopez
Copy link
Contributor Author

In order to prevent weird side effects from updating configuration from control message while editing the connector config, we'll need to also update the API calls to perform check and save at the same time.

For context, currently the connector creation and update pages perform two calls:

  • check_connection
  • update / create

Because check_connection can now also update the config, this might introduce unexpected results. We should combine the calls into one so we perform the update as expected.

@bazarnov
Copy link
Collaborator

bazarnov commented Jan 2, 2023

@pedroslopez Is there any ETA for this one?

@pedroslopez
Copy link
Contributor Author

@bazarnov The PR is in review #20894

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants