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

Clean up dynamic configs #2877

Merged

Conversation

alexshtin
Copy link
Member

@alexshtin alexshtin commented May 20, 2022

What changed?
Clean up dynamic configs.

Why?
Previous values were similar to defaults and there is no reason to set them explicitly in dynamic configs. Default values for advanced visibility are also auto configured based on static configs. docker.yaml is an empty file for default docker configuration.

Related PRs:
temporalio/docker-builds#50
temporalio/docker-compose#108

How did you test it?
Run different configurations locally. Build docker image locally and run it with docker-compose.

Potential risks
No risks. All configs in this repo are just for server developers.

Is hotfix candidate?
No.

system.advancedVisibilityWritingMode:
- value: "off"
constraints: {}
#frontend.enableClientVersionCheck:
Copy link
Contributor

@jbreiding jbreiding May 20, 2022

Choose a reason for hiding this comment

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

This might be a good time to include some sort of link to docs as a comment?

Instead of a structure that might go stale?

But maybe just for docker-compose repo?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is for server developers only. They know where to look (code).

@@ -117,5 +117,5 @@ publicClient:
hostPort: "localhost:7233"

dynamicConfigClient:
filepath: "config/dynamicconfig/development_es.yaml"
filepath: "config/dynamicconfig/development_cass.yaml"
pollInterval: "10s"
Copy link
Contributor

@jbreiding jbreiding May 20, 2022

Choose a reason for hiding this comment

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

this might be a good time to fix this by adding a newline

@alexshtin alexshtin enabled auto-merge (squash) May 20, 2022 17:30
@alexshtin alexshtin merged commit 98f4525 into temporalio:master May 20, 2022
@alexshtin alexshtin deleted the feature/cleanup-dynamic-configs branch May 20, 2022 17:34
Sushisource pushed a commit to Sushisource/temporal that referenced this pull request Jun 7, 2022
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.

None yet

2 participants