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

Allowing configuring UI codec URL via flag #54

Merged
merged 2 commits into from
Jan 5, 2023
Merged

Allowing configuring UI codec URL via flag #54

merged 2 commits into from
Jan 5, 2023

Conversation

feedmeapples
Copy link
Contributor

@feedmeapples feedmeapples commented Dec 13, 2022

What was changed

Added flag --ui-codec-endpoint for configuring codec endpoint used by UI

copy of temporalio/temporalite-archived#174

resolve #52

Why?

enables the use of codec via a single run comand

Checklist

  1. Closes

  2. How was this tested:

Manually

  1. Any docs updates needed?

server/ui.go Outdated
TemporalGRPCAddress: c.TemporalGRPCAddress,
EnableUI: c.EnableUI,
Codec: uiconfig.Codec{
Endpoint: c.CodecEndpoint,
Copy link
Member

Choose a reason for hiding this comment

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

Is it okay if this is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea the default value is empty. This means it's disabled

server/ui.go Outdated
return cfg, nil
}

type uiConfig struct {
Copy link
Member

Choose a reason for hiding this comment

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

Why not use uiconfig.Config instead of creating this new struct and renaming NewUIConfig to MergeWithConfigFile?
This is a bit confusing to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point

@feedmeapples feedmeapples merged commit 72653e6 into main Jan 5, 2023
@feedmeapples feedmeapples deleted the codec branch January 5, 2023 02:27
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.

[Feature Request] Apply https://github.com/temporalio/temporalite/pull/174
3 participants