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

feat(consul,service_tags): Support escaped comma #401

Merged

Conversation

samber
Copy link
Contributor

@samber samber commented Apr 20, 2016

#390

Example:

docker run -d \
      -p 4242:4242 \
      -e 'SERVICE_4242_TAGS=traefik.frontend.rule=Host: www.example.com\, Path: /api,traefik.enable=true' \
      company/service

Create only 2 tags into consul:

  • traefik.frontend.rule=Host: www.example.com, Path: /api
  • traefik.enable=true

@samber samber force-pushed the REGISTRATOR-390--escape-comma-tag-values branch 2 times, most recently from 23cd7c5 to 2698c7b Compare May 17, 2016 14:35
@josegonzalez
Copy link
Member

I'm not sure about this code, mostly because there are no tests for this utility file and this seems like a good candidate for tests. @samber Mind adding tests? Once you do, I can totally merge this in for the next release :)

@samber samber force-pushed the REGISTRATOR-390--escape-comma-tag-values branch from 2698c7b to 72f93b2 Compare June 30, 2016 09:41
@samber samber force-pushed the REGISTRATOR-390--escape-comma-tag-values branch from 72f93b2 to 6cff9a4 Compare June 30, 2016 09:45
@samber
Copy link
Contributor Author

samber commented Jun 30, 2016

Done ;-)

Please note #439 :trollface:

@jakew
Copy link

jakew commented Sep 28, 2016

Is there an ETA on when this would actually be merged or released? I look forward to this for using Traefik.

@josegonzalez
Copy link
Member

@jakew no ETA at the moment. I'm not sure anyone knows how to make a release of registrator at the moment, which I'm trying to sort out (amongst other things).

@bweston92
Copy link

Any update? I want to switch to Traefik and keep using this nice bit of kit :D

@Kemyke
Copy link

Kemyke commented Jan 20, 2017

This issue makes the registrator useless if you try to use it with traefik. Can somebody merge the PR? Its a quite small change and it is waiting for 6 months. Or should we consider this project dead and try to fix it ourselfs?

@mattatcha mattatcha merged commit 1a1a9ae into gliderlabs:master Jan 20, 2017
arentrue pushed a commit to rbkmoney/registrator that referenced this pull request Jan 25, 2017
* feat(service_tags): Support escaped comma
* test(bridge): Adds test for escaped comma in tags + fix types_test.go tests
lewispeckover pushed a commit to lewispeckover/registrator that referenced this pull request May 14, 2017
* feat(service_tags): Support escaped comma
* test(bridge): Adds test for escaped comma in tags + fix types_test.go tests
@ViViDboarder
Copy link

FYI for anyone affected by this: I just spent a while debugging issues with this and realized it's been merged but it's not part of the latest tag on Docker Hub.

https://hub.docker.com/r/gliderlabs/registrator/tags/

Make sure you're using gliderlabs/registrator:master.

@saurabhguptag
Copy link

could you please push this to "latest" tag do not want to use master image

@josegonzalez
Copy link
Member

@saurabhguptag We'll get to it when we get to it.

@gliderlabs gliderlabs locked and limited conversation to collaborators Aug 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants