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

Add support for the new fetcher cache in Mesos 0.23 #1953

Closed
ssk2 opened this issue Aug 6, 2015 · 25 comments
Closed

Add support for the new fetcher cache in Mesos 0.23 #1953

ssk2 opened this issue Aug 6, 2015 · 25 comments
Milestone

Comments

@ssk2
Copy link
Contributor

ssk2 commented Aug 6, 2015

It looks like this requires additional configuration and for the fetcher cache boolean to set to be true when tasks are executed (as per https://github.com/apache/mesos/blob/master/docs/fetcher.md). It would be nifty if Marathon supported this feature!

This is related to #1377.

@ssk2 ssk2 added the mesos label Aug 6, 2015
@pdiorio
Copy link

pdiorio commented Aug 6, 2015

+1 This would be a very useful addition!

@mindscratch
Copy link

👍

@sybrandy
Copy link

sybrandy commented Aug 6, 2015

idea++ 👍

@aquamatthias aquamatthias added this to the Backlog milestone Aug 12, 2015
@mindscratch
Copy link

Any chance this will make it in the 0.13.0 release?

@mindscratch
Copy link

I guess not. Perhaps 0.14?

@wendigo
Copy link
Contributor

wendigo commented Dec 21, 2015

I can work on this but I need a tip from core-devs how to expose it via API :)

Looking at source in V2AppDefinition: uris: Seq[String] = AppDefinition.DefaultUris, this cannot be extended to be backwards compatible. We can add another field that will be combined with uris though.

@wendigo
Copy link
Contributor

wendigo commented Dec 21, 2015

@aquamatthias looking forward for your opinion on that :)

@aquamatthias
Copy link
Contributor

The URI's in mesos look like this:

 message URI {
    required string value = 1;
    optional bool executable = 2;
    optional bool extract = 3 [default = true];
    optional bool cache = 4;
  }

In order to support this data structure, we have to move away from simple strings.
The new field should allow expressing those properties, where all properties than the URI are optional. Example:

[
  { "uri": "http://example.com/bla" },
  { "uri": "http://example.com/foo", "executable": true, "extract": false, "cache": false }
]

To make this backward compatible, the uris property should stay as is with defaults applied.

Since uris is a pretty good name, we need another one.
What would be a good name?

@wendigo
Copy link
Contributor

wendigo commented Dec 22, 2015

Maybe something like "artifacts" since we are downloading them in order to launch our application.

@wendigo
Copy link
Contributor

wendigo commented Dec 22, 2015

Or "fetchUris" or "downloadUris".

@mindscratch
Copy link

I'd prefer uris change to new format. As you mentioned, today there are default values being used for the other properties, why not expose them? There have been backward incompatible changes made before.

@wendigo
Copy link
Contributor

wendigo commented Dec 22, 2015

@mindscratch I do not like breaking compatibility if it can be avoided.

@aquamatthias
Copy link
Contributor

We already also have storeUrls - which makes it even more confusing...
At least in the format we should be backward compatible.

@mindscratch
Copy link

So what would be the difference between uris and say, fetchUris? It seems confusing to have two different properties that might do the same thing.

@wendigo
Copy link
Contributor

wendigo commented Dec 22, 2015

I think that best solution is to provide V3 API in which every primitve type (i.e. string) is replaced with class enclosing this type (so "uris": ["uri1"] becomes "uris": [{"uri": "uri1"}]). Since we can't do that I would introduce new field and mark old one as deprecated to be dropped in next versions.

@wendigo
Copy link
Contributor

wendigo commented Dec 27, 2015

Any thoughts @aquamatthias ?

@aquamatthias
Copy link
Contributor

Hey @wendigo had the same idea in mind (see comment above) - introduce a new parameter with new format which supersedes 'uris'.

@wendigo
Copy link
Contributor

wendigo commented Dec 27, 2015

So let's settle on naming and I can implement this feature :)

@mindscratch
Copy link

I'll vote for fetchUris ...can't wait to see this land.
On Dec 27, 2015 1:35 PM, "Mateusz Gajewski" notifications@github.com
wrote:

So let's settle on naming and I can implement this feature :)


Reply to this email directly or view it on GitHub
#1953 (comment)
.

@aquamatthias
Copy link
Contributor

Naming is hard ...
What about the simple version?
fetch:[{uri:xxx}]

Am 27.12.2015 um 22:10 schrieb Craig Wickesser notifications@github.com:

I'll vote for fetchUris ...can't wait to see this land.
On Dec 27, 2015 1:35 PM, "Mateusz Gajewski" notifications@github.com
wrote:

So let's settle on naming and I can implement this feature :)


Reply to this email directly or view it on GitHub
#1953 (comment)
.


Reply to this email directly or view it on GitHub.

@wendigo
Copy link
Contributor

wendigo commented Dec 28, 2015

Ok, I'm on it :)

@mindscratch
Copy link

Sounds good
On Dec 28, 2015 11:13 AM, "Mateusz Gajewski" notifications@github.com
wrote:

Ok, I'm on it :)


Reply to this email directly or view it on GitHub
#1953 (comment)
.

@wendigo
Copy link
Contributor

wendigo commented Jan 5, 2016

Almost there 😄

Tests: succeeded 849, failed 0, canceled 0, ignored 2, pending 0

@mindscratch
Copy link

Awesome!

@wendigo
Copy link
Contributor

wendigo commented Jan 19, 2016

Done!

@aquamatthias aquamatthias modified the milestones: 0.15.0, Backlog Jan 19, 2016
@mesosphere mesosphere locked and limited conversation to collaborators Mar 27, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants