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

Backport PR #813 to 1.1.7 series #815

Closed
wants to merge 1 commit into from
Closed

Backport PR #813 to 1.1.7 series #815

wants to merge 1 commit into from

Conversation

daltonmatos
Copy link
Contributor

Hi,

I would like to know if it would be possible to backport this PR (#813) to Marathon UI 1.1.x series. I will try to explain the reasoning behind this.

We run our infra-structure (currently) fully based on Apache Mesos and Mesosphere Marathon for orchestration of every task we run. We run Marathon backend without the UI and deploy the UI separately (it's indeed managed by marathon itself). This gives us the ability to evolve the UI (write plugins, changing code, etc) without having to mess with the Marathon backend process.

We run Marathon backend version 1.1.1 and are in the process to update to 1.3.13. And Marathon UI 1.1.7 is the last version of the UI that is compatible with Marathon backend 1.3.13 (After 1.1.7 there were introduced some incompatible changes, eg:
76d1eab. In fact, we are already running UI version 1.1.7 talking to Backend version 1.1.1 and will soon update to backend 1.3.13.

Since the modification made by PR #813 is unrelated to other areas of the code and is a self-contained implementation, I though it would be OK to backport this to 1.1.x series and release a new "minor version" for this series, I don't know if it would be 1.1.7.1 or 1.1.8. I see also that some commits mention 1.1.8 so maybe would be better to jump to 1.1.9 instead.

Until we update our backend to marathon 1.4.x or 1.5.x we can't update our UI also. That's why it would be immensely useful if we could just forward our UI to 1.1.9 so we would be able to use this new interface and some of our plugins would directly benefit from this implementation.

We are currently running our UI from the 1.1.7 branch, with PR #813 applied (plus other changes that we plan to open future PRs), but would be much better not to have a too much diverged code base from the original project. That's why we started to contribute to the Marathon UI project in a way that we change the core to make it possible to write better plugins. That's how we are using Marathon currently and plan to customize it to our needs, but always making the project stronger and more useful to others.

So I would like to ask you to consider merging this code and releasing a new minor version for 1.1.x series. it would be much valuable.

About how I did this PR, I only cherry-picked the merge commit from master. I don't know if this is the best way to do it. If you are willing to confirm this merge+release I would adjust anything needed, just let me know.

Thank you very much.

…-config-available-to-plugins

Exporting ajaxWrapper and config to UI plugins
@daltonmatos
Copy link
Contributor Author

@orlandohohmeier

@daltonmatos
Copy link
Contributor Author

Hello @orlandohohmeier, any thoughts?

Thanks,t

@orlandohohmeier
Copy link
Contributor

orlandohohmeier commented Oct 16, 2017

Hi @daltonmatos,

I hesitate to backport the feature as it requires us to violate the semantic version guidelines and wonder what we can instead do to help you update to a newer Marathon version or switch to DC/OS?

I'm ok to move ahead and backport the feature if there are others who need this feature in 1.1.7+ and the community is ok with a backport even if that means we're violating the guidelines. Please reach out to the community and ask for feedback to move this one forward.

On another note, please cherry-pick the actual commits (c8e0e04...b780383) not the merge-commit so that's easier to track the changes.

As always, thanks for pushing and investing into Marathon UI!
Orlando

/cc @meichstedt

@daltonmatos
Copy link
Contributor Author

daltonmatos commented Oct 16, 2017

Hello @orlandohohmeier,

Sorry for violating marathon UI versioning guidelines. Indeed I didn't know such guidelines existed. Are these documented somewhere? I would like to know them better.

What would be the best channel to reach out to the community? the marathon-framework mailing list? I think nobody will really care about this backport, since this is a very experimental change and probably I'm the only one really using it. 😄

This won't be such a problem because I can backport this change myself and devire my version of Marathon UI off of this branch, until I'm able to update to Marathon 1.4.x. No big deal, actually. The idea behind this PR is that it would be more secure to branch off of an official branch.

About the upgrade to a newer Marathon. This is totally doable, we are just going babysteps. First we are moving from 1.1.1 to 1.3.13. And then we will have a tricky update, because Marathon 1.4.x is already incompatible with the UI we are (and will be) using, so we will have to update both at the same time.

Since our UI is deployed separated and is currently modified, we can't use the bundled UI that comes with Marathon backend, and we also can't rollback the update since Marathon 1.4.x will migrate zookeeper nodes in a way that Marathon 1.3.x won't understand (I'm thinking about the network related changes, container.docker.network being moved to networks, for example), am I right?

We are being very careful about the future upgrade to Marathon 1.4.x (and after this, to 1.5.x) but we definitely want (and eventually will) migrate.

About DC/OS I still have to study it more deeply and check if it support everything we are currently adapting Marathon to do for us.

Thanks,

@daltonmatos
Copy link
Contributor Author

Hello @orlandohohmeier,

I'm closing this PR because I will do the backport myself, until I update to UI 1.2.x.

Thanks a lot for your help! There are more PR that I wish to open about the Plugin infrastrucure. I will work on these and post them back here no github.

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
2 participants