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

Fixing deployment events handling for apps in groups #40

Merged
merged 1 commit into from
Jan 28, 2016

Conversation

dankraw
Copy link
Contributor

@dankraw dankraw commented Jan 27, 2016

fixes #35

@janisz
Copy link
Contributor

janisz commented Jan 27, 2016

Custom names looks for the first sight looks easy, but now we get more and more complicated code. There could be more corner cases than we previously thought. We need to discuss this feature.
I'll test this PR tomorrow, code looks good 👍

@adamdubiel
Copy link
Collaborator

Maybe we should pivot now and make it as simple as possible. @dankraw suggested some simple k/v mapping, Consul has own k/v so why not use it if we go this way.

@dankraw
Copy link
Contributor Author

dankraw commented Jan 28, 2016

OK, so as we spoke, we're gonna keep the current implementation for v0.3.x and maybe return to the KV-store idea in the future if there are more issues with parsing large events / weird corner-cases etc.
The KV-store integration is not cheap either as we thought at first. The data has to be put there depending on some stimulus/app configuration, it needs to be replicated to all datacenters, plus we would have to rethink how to support renaming of services.

@dankraw dankraw force-pushed the apps_in_groups_deployments branch 3 times, most recently from 8f3968c to 29e29ab Compare January 28, 2016 12:30
dankraw added a commit that referenced this pull request Jan 28, 2016
Fixing deployment events handling for apps in groups
@dankraw dankraw merged commit ac8ede2 into master Jan 28, 2016
@dankraw dankraw deleted the apps_in_groups_deployments branch January 28, 2016 13:13
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.

Not handling properly deployment events for apps with custom parent groups in Marathon
3 participants