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

Added devices config handling and device HostConfig handling #1400

Merged
merged 1 commit into from
May 11, 2015

Conversation

DanElbert
Copy link

I'm new to contributing to compose, but this is a stab at adding the device option to the yml config, as per issue #754.

@dnephin
Copy link

dnephin commented May 9, 2015

I think this is a good start. I believe we also need to update compose/config.py to be able to merge the lists of devices when using extends

@DanElbert
Copy link
Author

I've updated the pull request.

@@ -342,6 +342,15 @@ dns_search:
- dc2.example.com
```

### devices

Devices option. List of device mappings. Uses the same format as the --device docker client create option.
Copy link

Choose a reason for hiding this comment

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

  • First sentence is unnecessary
  • --device should be in backticks
  • Wrap to 80 chars please!

@aanand
Copy link

aanand commented May 11, 2015

Thanks!

For the sake of consistency, we should support a mapping as well as a list. Is that supported? If so, it should be documented.

@aanand
Copy link

aanand commented May 11, 2015

For the sake of consistency, we should support a mapping as well as a list.

I retract this, as I was thinking about environment variables. We don't support YAML mappings for volumes. Sorry for the noise.

This looks good - just needs squashing to a single commit.

@DanElbert
Copy link
Author

Thanks for the feedback. I was just going to ask about the mapping. :)

Signed-off-by: Dan Elbert <dan.elbert@gmail.com>
@DanElbert DanElbert force-pushed the 754-device_option branch from 53a08c0 to df87bd9 Compare May 11, 2015 15:52
@DanElbert
Copy link
Author

All squashed. Let me know if I missed anything else.

@aanand
Copy link

aanand commented May 11, 2015

LGTM

1 similar comment
@dnephin
Copy link

dnephin commented May 11, 2015

LGTM

dnephin added a commit that referenced this pull request May 11, 2015
Added devices config handling and device HostConfig handling
@dnephin dnephin merged commit 4997fac into docker:master May 11, 2015
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.

4 participants