-
Notifications
You must be signed in to change notification settings - Fork 339
feat(temporal): allow customization of SSL mode for external db #361
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
Conversation
Signed-off-by: CyberHippo <[email protected]>
|
hi @davinchia, do you have an ETA ? 🙏 |
|
Thanks @CyberHippo for opening this. We too are broke by the previous change. Looking forward to this. |
|
Hello all, sorry the delay to get review in this contribution. I raised it to the platform team. After I get a return will update here. |
|
Hi @marcosmarxm, do you have some news about this blocking issue for us ? 🙏 |
|
Any news on when this PR is going to be merged? It is blocking for me too. I am setting up Airbyte with Flux, so the workaround to adjust the variables will not work for me, as they will be overwritten again. |
|
Following the comment from @PurseChicken, do we still need this PR? |
Hi @CyberHippo |
|
Hi @CyberHippo, do you have an ETA for the fix release ? 🙏 |
|
Hi @kev-datams , unfortunately I am not part of Airbyte platform team and don't know about their chart release process. I am also waiting for this to be released. Maybe @davinchia knows more about it? |
|
Apologies for the missing updates on this contribution. I'll coordinate with Davin to determine the next steps for securing a review. |
Thanks @marcosmarxm 🙏 |
## What <!-- * Describe what the change is solving. * It helps to add screenshots if it affects the frontend. --> Issue: airbytehq/airbyte#54176 Current Helm `values.yml` documentation and defaults have old unused values for `externalDatabase`, potentially leading to confusion when setting up or upgrading Helm chart versions. ## How <!-- * Describe how code changes achieve the solution. --> This removes `externalDatabase` from all defaults and documentation. These values are not used anywhere in the project outside of the changes files in this PR. Instead `global.database` should be used instead to configure external PostgreSQL connections. ## Can this PR be safely reverted and rolled back? <!-- * If you know that your be safely rolled back, check YES.* * If that is not the case (e.g. a database migration), check NO. * If unsure, leave it blank.* --> - [x] YES 💚 - [ ] NO ❌ Co-authored-by: perangel <[email protected]>
|
Do we have any updates on this? We are currently facing challenges deploying Airbyte with an external database that requires SSL. While we have managed to get most of the features working, we are stuck on an unresolvable issue with the pods spawned by the airbyte-workload-launcher. The problem lies in the Helm templates for these child pods, where the extraVolumeMounts and extraVolumes declarations are missing. As a result, although Airbyte can partially work with an external database using SSL — thanks to the extraEnv, extraVolumeMounts, and extraVolumes settings being configurable through the values.yml — the child pods are unable to inherit these configurations. Any guidance or updates would be greatly appreciated. |
|
Hi, we've decided to stop accepting PRs in this repo, so we can focus on supporting our connector contributors. I'm going to close this issue. We appreciate your work here. Please feel free to post an issue in https://github.com/airbytehq/airbyte. |
|
Just creating this new issue following your message @cgardens : airbytehq/airbyte#64156 |
What
Due to a recent chart change, it is not possible to disable SSL for external databases.
This change to the temporal chart allows to configure the SSL value for both internal and external databases.
Fixes airbytehq/airbyte#43328
How
By adding a new chart value to enable or disable SSL for database in the
globalsection.This parameter is not related to the database type.
Note: The new value could be moved to the
temporalsection if needed. I thought that since the SSL flags were initially introduced in the global section I would add the new value there as well:Recommended reading order
Can this PR be safely reverted and rolled back?