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

Read TOXIPROXY_* environment variables if they exist as default values for the hostname #319

Merged
merged 2 commits into from Sep 17, 2021

Conversation

maaslalani
Copy link
Contributor

@maaslalani maaslalani commented Sep 16, 2021

This PR introduces reading from the TOXIPROXY_URL variable (or TOXIPROXY_HOST + TOXIPROXY_PORT) as defaults rather than needing to pass in the -h somehost.railgun:8474 each time any command is run.

No variable exported

This tries to read the normal fallback url and cannot connect since I don't have toxiproxy running on this url.

$ go run cli/cli.go list
Failed to retrieve proxies: Get "http://localhost:8474/proxies": dial tcp 127.0.0.1:8474: connect: connection refused
exit status 1

Variable exported

Connects to the correct host without the extra flags, even though it is not 127.0.0.1:8474 since the TOXIPROXY_URL is set.

$ export TOXIPROXY_URL="http://somehost.railgun:8474"
$ go run cli/cli.go list
Name                    Listen          Upstream                Enabled         Toxics
======================================================================================
mysql                   192.168.64.48:30500     somehost.railgun:3306        enabled         1

Hint: inspect toxics with `toxiproxy-cli inspect <proxyName>`

@miry miry self-assigned this Sep 17, 2021
@miry
Copy link
Contributor

miry commented Sep 17, 2021

@maaslalani Hi. It looks ok. Did you check the feature of urfave/cli to read values from ENVs: https://github.com/urfave/cli/blob/master/docs/v2/manual.md#values-from-the-environment?

Would it better to have the framework handle all configurations?

cli/cli.go Outdated Show resolved Hide resolved
@miry miry assigned maaslalani and unassigned miry Sep 17, 2021
@maaslalani
Copy link
Contributor Author

@maaslalani Hi. It looks ok. Did you check the feature of urfave/cli to read values from ENVs: https://github.com/urfave/cli/blob/master/docs/v2/manual.md#values-from-the-environment?

Would it better to have the framework handle all configurations?

Ah no I didn't, I'll change this up to do that, sounds like a much better approach, thanks!

@maaslalani
Copy link
Contributor Author

@miry Updated this PR, using the EnvVars is much better 👍 Thanks for the suggestion.

@miry
Copy link
Contributor

miry commented Sep 17, 2021

@maaslalani Pls update changelog.

CHANGELOG.md Outdated Show resolved Hide resolved
@maaslalani maaslalani merged commit 3d6882e into master Sep 17, 2021
@maaslalani maaslalani deleted the ml-read-from-environment branch September 17, 2021 14:35
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