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 to compose syntax for volumes_from, volumes, and optional cpu parameter #42

Conversation

philipn
Copy link
Contributor

@philipn philipn commented Apr 5, 2016

  • The cpu parameter in ECS is now optional. I think it was required before. So we don't emit an error message when it's not present.
  • Allow the compose v2 style 'service:name:ro' syntax in volumes_from clauses.
  • Allow the compose style ':rw' in volumes clauses.
  • Emit readOnly on ECS volumesFrom clauses, which were previously skipped.

  * The `cpu` parameter in ECS is now optional.  I think it was required
    before.  So we don't emit an error message when it's not present.

  * Allow the compose v2 style 'service:name:ro' syntax in
    `volumes_from` clauses.

  * Emit `readOnly` on ECS `volumesFrom` clauses, which were previously
    skipped.
@cjerdonek
Copy link

Love it! 👍

@philipn philipn changed the title Support new compose syntax for volumes_from & make cpu optional. Fixes to compose syntax for volumes_from, volumes, and optional cpu parameter Apr 5, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ab869b2 on philipn:support_rwo_volumes_from_and_cpu_optional into ff5741c on micahhausler:master.

parts = vol.split(':')
rwo_value = None

assert(len(parts) <= 3)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add an explanation parameter to this assert so the error is a little more clear? ``assert(len(parts) <=3, "volume string '{}' has too many colons".format(vol))` or something like that?

@micahhausler
Copy link
Owner

This is great! Thanks for submitting. Just one comment on the compose ingest_volumes_from, and we're good to go.

@micahhausler
Copy link
Owner

@philipn Any chance you could fix the indentation for the failing CI?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling f2890b3 on philipn:support_rwo_volumes_from_and_cpu_optional into 8a3ed35 on micahhausler:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 330dcc4 on philipn:support_rwo_volumes_from_and_cpu_optional into 224a596 on micahhausler:master.

@sveneh
Copy link

sveneh commented Dec 15, 2016

any blockers for this to be merged?

@micahhausler micahhausler merged commit d29a079 into micahhausler:master Dec 15, 2016
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.

None yet

5 participants