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.

Sign up for GitHub

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

Jump to bottom

Fixes for state storage env vars #22191

Merged
merged 4 commits into from
Feb 7, 2023
Merged

Fixes for state storage env vars #22191

merged 4 commits into from
Feb 7, 2023

Conversation

adam-bloom
Copy link
Contributor

@adam-bloom adam-bloom commented Feb 1, 2023 •
edited
Loading

What

  • The current env var checks for state storage do not properly exclude empty env vars, as is currently implemented for log storage. This means that it is impossible to configure S3 state storage using both kustomize and helm, as those define env vars for the minio state storage (even when they are empty)
  • S3 state storage had two different env vars used for region

How

  • Borrows the same logic pattern from log storage to apply to state storage
  • Consolidate to a single env var for S3 state storage region (these env vars are not currently set by either kustomize or helm)

Recommended reading order

  1. x.java
  2. y.python

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

@octavia-squidington-iv octavia-squidington-iv added area/platform issues related to the platform area/server area/worker Related to worker community labels Feb 1, 2023
@adam-bloom adam-bloom changed the title Fixes for a state storage env vars Fixes for state storage env vars Feb 1, 2023
@adam-bloom adam-bloom mentioned this pull request Feb 2, 2023
Closed
37 tasks
@davinchia davinchia requested review from xpuska513 and davinchia and removed request for xpuska513 February 2, 2023 17:08
@davinchia davinchia merged commit 34ecd83 into airbytehq:master Feb 7, 2023
@github-actions github-actions bot mentioned this pull request Feb 7, 2023
Closed
@davinchia davinchia mentioned this pull request Feb 8, 2023
Merged
37 tasks
davinchia added a commit that referenced this pull request Feb 8, 2023
@davinchia
Kustomize: Set up S3 state config variables (#22537)
4180b57 
Kustomize charts are missing this today. OSS users are running into this and complaining.

Application logic fixed in #22191.

This PR fixes the Kustomize deployments.
@octavia-squidington-iii octavia-squidington-iii mentioned this pull request Feb 8, 2023
Closed
danidelvalle pushed a commit to danidelvalle/airbyte that referenced this pull request Feb 9, 2023
@davinchia @danidelvalle-frontiers
Kustomize: Set up S3 state config variables (airbytehq#22537)
7dad6a7 
Kustomize charts are missing this today. OSS users are running into this and complaining.

Application logic fixed in airbytehq#22191.

This PR fixes the Kustomize deployments.
This was referenced Feb 10, 2023
Closed
Closed
@malikdiarra malikdiarra mentioned this pull request Jul 7, 2023
Closed
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davinchia davinchia davinchia approved these changes

Assignees
No one assigned
Labels
area/platform issues related to the platform area/server area/worker Related to worker community
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@adam-bloom @davinchia @octavia-squidington-iv