Skip to content

Conversation

@jjlgao
Copy link
Contributor

@jjlgao jjlgao commented Jun 5, 2025

No description provided.

@jjlgao jjlgao force-pushed the jason/fix-base-domain-requirement branch 3 times, most recently from 6c9c1ee to 79ee991 Compare June 6, 2025 15:10
@jjlgao jjlgao force-pushed the jason/fix-base-domain-requirement branch from 79ee991 to f8e6c99 Compare June 6, 2025 15:14
env: {}
env:
- name: BASE_DOMAIN
value: "CHANGE_ME"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can add this as a comment, but we shouldn't set it as a default. for any pre-existing deployment on an older version, if it didn't have a BASE_DOMAIN defined before and then the chart gets updated to include this default env value, the retool backend will start rejecting requests I think. that's worse than just leaving out a default BASE_DOMAIN even though it's required, and letting the pod error with an informative error message until the operator sets a value.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryanartecona Can we add it but comment it out? Then highlight in the file itself that this env var is a requirement and users should uncomment it and update it to their base_domain.

@jjlgao
Copy link
Contributor Author

jjlgao commented Jun 6, 2025

Superseded by #239

@jjlgao jjlgao closed this Jun 6, 2025
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

Successfully merging this pull request may close these issues.

4 participants