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

Use names already supported by our tools #210

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft

Conversation

robholland
Copy link
Collaborator

@robholland robholland commented May 29, 2024

What was changed

Deprecated some variable names in favour of clearer alternatives.

Why?

This avoids naming that is confusing users new to deploying Temporal, and reduces the amount of mapping needed between the environment and our tools (which already accept the environment variables I'm proposing we use here).

As an added small bonus we get full connection attribute support rather than just MySQL transaction isolation.

The rollout plan would be:

  1. Update our docker config template to support the new variables as-is in our container images, this needs to happen before this PR is merged.
  2. Remove the deprecation message for now, but keep the auto-migration
  3. Update our docker-compose files and internal systems to use the new variables
  4. Turn on the deprecation messages.
  5. After some time, remove the old variables.

Note that the auto-setup.sh script only supports changing the database name between persistence and visibility configurations. Our config template supports a wider range of visibility overrides, which we should also support in auto-setup.sh. This PR does not attempt to address that now though, and rather aims to keep functionality the same, aside from the connection attribute support mentioned above.

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

docker/auto-setup.sh Outdated Show resolved Hide resolved
@robholland robholland marked this pull request as ready for review May 29, 2024 17:12
@robholland robholland requested a review from a team as a code owner May 29, 2024 17:12
@alexshtin alexshtin changed the title Use names already supported by our tools. Use names already supported by our tools Jun 4, 2024
@tdeebswihart tdeebswihart requested a review from alexshtin June 5, 2024 14:45
@robholland robholland marked this pull request as draft June 12, 2024 11:18
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.

1 participant