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

Fixes #72 | Unregister only tasks for given apps #73

Merged
merged 1 commit into from
Apr 22, 2016

Conversation

janisz
Copy link
Contributor

@janisz janisz commented Apr 12, 2016

This PR intorduce logic in killing stoped application. Only tasks from killed/stoped application will be deregistered.

This fixes #72 #35 #42

@dankraw
Copy link
Contributor

dankraw commented Apr 12, 2016

👍

@dankraw
Copy link
Contributor

dankraw commented Apr 12, 2016

You can update the readme as well. :)

@dankraw
Copy link
Contributor

dankraw commented Apr 13, 2016

OK, I tested it and there are issues when deregistering services after app removal. :(

@janisz janisz force-pushed the fixes_#72_unregister_only_tasks_for_given_apps branch from 2a3d2bb to 8a1e0b6 Compare April 13, 2016 12:47
@janisz
Copy link
Contributor Author

janisz commented Apr 13, 2016

My bad, missing / in the begining of AppId causes no task was deregistered

@janisz janisz force-pushed the fixes_#72_unregister_only_tasks_for_given_apps branch from 8a1e0b6 to 9123639 Compare April 13, 2016 13:14
@dankraw
Copy link
Contributor

dankraw commented Apr 22, 2016

It works great now! :)

@janisz janisz merged commit 9843a5d into master Apr 22, 2016
@janisz janisz deleted the fixes_#72_unregister_only_tasks_for_given_apps branch April 22, 2016 13:27
@dankraw
Copy link
Contributor

dankraw commented Apr 22, 2016

We can remove
"Important: it should be unique for every Marathon cluster connected to Consul."
"Every marathon application needs to have a unique service name in Consul."
from readme.

@janisz
Copy link
Contributor Author

janisz commented Apr 22, 2016

I create new pr for README update #79

@dankraw
Copy link
Contributor

dankraw commented Apr 22, 2016

cool thanks :)

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 multiple marathon apps with same consul service name
3 participants