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

Fix memory hog during image import #245

Closed
wants to merge 28 commits into from

Conversation

shurkaxaa
Copy link
Contributor

Existing version load image into memory when non-unified image used
(with separate tarball with manifest).
Added ability for stream based image upload without loading them to memory.
requests-toolbelt external package required for streaming multipart HTTP
image upload.
Additional parameter added to identify if streams are/can be passed - this will allow
pylxd client to identify via introspection if pylxd library supports stream based
upload and fallback to old non-stream behavior if old version of pylxd is installed.

atrautsch and others added 20 commits February 4, 2017 22:48
This adds basic support for storage pools as well
as tests to verify their function, taken from the
LXD REST API.
memory. requests-toolbetl external package required as embedded
to requests multipart 'files' based upload still load parts to
memory.
memory. requests-toolbetl external package required as embedded
to requests multipart 'files' based upload still load parts to
memory.
This adds support for LXD being installed via snap
in addition to the traditional apt package.

Fixes canonical#248
@ajkavanagh
Copy link
Contributor

I think the tests are failing because your branch is behind master. Please could you rebase your branch against pylxd/master and we can see if the tests will pass. Thanks.

ajkavanagh and others added 6 commits September 26, 2017 21:32
Allow pylxd to work with snap installed lxd as well as normally installed lxd from packages.  This is needed as the unixsocket is inside the snap at /var/snap/lxd/common/lxd/unix.socket.
Include created_at attribute for the snapshot object.
memory. requests-toolbetl external package required as embedded
to requests multipart 'files' based upload still load parts to
memory.
memory. requests-toolbetl external package required as embedded
to requests multipart 'files' based upload still load parts to
memory.
@codecov-io
Copy link

codecov-io commented Sep 27, 2017

Codecov Report

Merging #245 into master will increase coverage by 0.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
+ Coverage   95.93%   96.14%   +0.21%     
==========================================
  Files          14       15       +1     
  Lines         689      727      +38     
  Branches       78       81       +3     
==========================================
+ Hits          661      699      +38     
  Misses         14       14              
  Partials       14       14
Impacted Files Coverage Δ
pylxd/models/storage_pool.py 100% <100%> (ø)
pylxd/models/container.py 93.67% <100%> (+0.03%) ⬆️
pylxd/managers.py 100% <100%> (ø) ⬆️
pylxd/client.py 98.47% <100%> (+0.03%) ⬆️
pylxd/models/image.py 99.15% <100%> (+0.04%) ⬆️
pylxd/tests/models/__init__.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94210ae...ad5385b. Read the comment docs.

@shurkaxaa
Copy link
Contributor Author

Rebased as requested. Now CI tests passed.

@ajkavanagh
Copy link
Contributor

Hi. It looks like something is wrong with the PR from a rebasing perspective. We're going to need to fix this PR before it can merge because it's showing commits that were made on master in your PR. Did you do a merge rather than a rebase? What we would like in the PR is just the changes that the PR wants to make to the master branch, which I think are quite small. It might be easiest to fix this, but closing the PR (but make sure you don't lose your work) and, re-branching from master, and re-applying your changes and doing a new PR? Thanks.

@shurkaxaa
Copy link
Contributor Author

Ok, new pull request created #252. This PR should be closed.

@shurkaxaa shurkaxaa closed this Sep 27, 2017
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

6 participants