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

Add management of TF_APPEND_USER_AGENT #46

Merged
merged 1 commit into from
Aug 26, 2020
Merged

Add management of TF_APPEND_USER_AGENT #46

merged 1 commit into from
Aug 26, 2020

Conversation

paultyng
Copy link
Contributor

@paultyng paultyng commented Aug 7, 2020

Fixes #9

ua := mergeUserAgent(
os.Getenv(appendUserAgentEnvVar),
tf.appendUserAgent,
fmt.Sprintf("HashiCorp-terraform-exec/%s", version.ModuleVersion()),
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these values be in reverse order? i.e. static HashiCorp-terraform-exec first and then other values being appended, rather than prepended?

Copy link
Contributor Author

@paultyng paultyng Aug 14, 2020

Choose a reason for hiding this comment

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

I think it depends. arguably "terraform cloud" or "binary testing" (as the appendUserAgent) is more significant / distinct than terraform exec I think in the case of that system? As there will be many terraform exec apps, so the app itself is the more significant identifier?

Copy link
Member

Choose a reason for hiding this comment

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

I don't disagree, but the problem is that the ENV variable is called append, not prepend, so we would need to reflect it there somehow (with another/changed variable), else the behaviour won't match the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats the common convention in programming but not the english definition of the word. It was a poor original choice for the env var, but its what we have so no option to change it now.

@paultyng paultyng merged commit 9f4ccbe into master Aug 26, 2020
@paultyng paultyng deleted the ua branch August 26, 2020 12:58
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.

Support passing User-Agent
2 participants