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

fix issue #49938: missing property in helm airbyte/value.yaml (helm v1.3.1) #392

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sc-yan
Copy link

@sc-yan sc-yan commented Mar 10, 2025

What

there is a somewhat "breaking" change introduced by this commit. 02e3c97
I'm not sure the context of this commit, but as far as I understand, global.serviceAccountName should be used across each services in airbyte.
however, if the purpose is to let "workload-launcher" to use its own serviceaccount, then airbyte/values.yaml should be updated because this is where most end-users refer to since there is no changelog for helm charts.

How

to fix this issue airbytehq/airbyte#49938

Recommended reading order

  1. value.yaml

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Tybot204 and others added 2 commits February 28, 2025 12:45
## 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]>
@CLAassistant
Copy link

CLAassistant commented Mar 10, 2025

CLA assistant check
All committers have signed the CLA.

@sc-yan sc-yan changed the title fix issue #49938: missing property in helm value.yaml (helm v1.3.1) fix issue #49938: missing property in helm airbyte/value.yaml (helm v1.3.1) Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants