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

UI: Fix restoring state for service names containing slashes #988

Merged
merged 1 commit into from
Jun 2, 2015

Conversation

rafikk
Copy link
Contributor

@rafikk rafikk commented May 29, 2015

If a service name contains slashes, the web UI shows a blank screen.

@rafikk
Copy link
Contributor Author

rafikk commented Jun 1, 2015

Any feedback on this?

@ryanuber
Copy link
Member

ryanuber commented Jun 1, 2015

@rafikk we generally recommend against using any non-DNS compatible characters in service names. That said, as long as it's allowed in Consul, the UI should support it too. @pearkes any thoughts on this?

@rafikk
Copy link
Contributor Author

rafikk commented Jun 2, 2015

I agree and that's my perspective as well. The point of the UI is to show you state retrieved from Consul. We're successfully registering service names with slashes and utilizing the data via the agent API and consul-template. The only tool giving us trouble is the Consul UI.

as long as it's allowed in Consul, the UI should support it too

Can we merge this change then?

FWIW, the services we're registering are running in Apache Aurora where the convention is that they are named cluster/role/environment/service. Marathon also supports application groups which use hierarchical naming. I expect we are not the only users of Consul who expect this functionality.

@pearkes
Copy link
Contributor

pearkes commented Jun 2, 2015

@rafikk This should be fine to merge but I think @ryanuber is just waiting on me double checking Ember won't mind. Will do that shortly and give a 👍.

@rafikk
Copy link
Contributor Author

rafikk commented Jun 2, 2015

Sounds good. FYI, I did test this on our Consul cluster and all checked out.

@sethvargo
Copy link
Contributor

👍

@ryanuber
Copy link
Member

ryanuber commented Jun 2, 2015

Sounds like this is G2G, so I'm going to merge this in. Thanks, @rafikk!

ryanuber added a commit that referenced this pull request Jun 2, 2015
UI: Fix restoring state for service names containing slashes
@ryanuber ryanuber merged commit ee74f38 into hashicorp:master Jun 2, 2015
duckhan pushed a commit to duckhan/consul that referenced this pull request Oct 24, 2021
* Update CHANGELOG.md

* Fix unit test
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.

4 participants