Skip to content

Conversation

smothiki
Copy link
Contributor

Signed-off-by: sivaram mothiki smothiki@engineyard.com

There is no hard enforcement of syntax on specifying memory from docker side . I think docker-py should return an Error only if docker engine errors out and accept any syntax.

$ sudo docker run -d --name serame -m 30MB busybox sleep 30
$ docker inspect serame|grep Memory
        "Memory": 31457280,
        "MemorySwap": 0,
$ sudo docker run -d --name serame -m 30M busybox sleep 30
$ docker inspect serame|grep Memory
        "Memory": 31457280,
        "MemorySwap": 0,
$ sudo docker run -d --name serame -m 30Mb busybox sleep 30
$ docker inspect serame|grep Memory
        "Memory": 31457280,
        "MemorySwap": 0,
$ sudo docker run -d --name serame -m 30mb busybox sleep 30
$ docker inspect serame|grep Memory
        "Memory": 31457280,
        "MemorySwap": 0,
$ sudo docker run -d --name serame -m 30MK busybox sleep 30
FATA[0000] invalid size: '30MK'

@smothiki smothiki force-pushed the mems branch 2 times, most recently from 23fa2a1 to b0881c2 Compare April 24, 2015 20:00
@mboersma
Copy link

mboersma commented May 4, 2015

+1

@shin-
Copy link
Contributor

shin- commented May 7, 2015

Thank you for the contribution! Could you add unit tests to validate that this will behave as expected?

@smothiki smothiki force-pushed the mems branch 4 times, most recently from a7e9fe1 to 6bcdd40 Compare May 8, 2015 01:59
Signed-off-by: sivaram mothiki <smothiki@engineyard.com>
@smothiki
Copy link
Contributor Author

smothiki commented May 8, 2015

Apologies for multiple commits. Ignored checking flake8 every time

@smothiki
Copy link
Contributor Author

smothiki commented May 8, 2015

@shin- I have added integration tests. Let me know if you want more test cases

@shin-
Copy link
Contributor

shin- commented May 20, 2015

Thanks!

shin- added a commit that referenced this pull request May 20, 2015
make memory units compatible with native docker cli
@shin- shin- merged commit 7b2fd8c into docker:master May 20, 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.

3 participants