Skip to content
This repository has been archived by the owner on Feb 19, 2024. It is now read-only.

Package instances can only be used once #14

Closed
javabrett opened this issue Mar 22, 2016 · 4 comments
Closed

Package instances can only be used once #14

javabrett opened this issue Mar 22, 2016 · 4 comments

Comments

@javabrett
Copy link

If you create a Package object:

full_path = "my_1.1.1_amd64.deb"
pkg = Packagecloud::Package.new(:file => full_path)

Then attempt to use it twice:

$client.put_package("my", pkg, ""ubuntu/trusty")
$client.put_package("my", pkg, ""ubuntu/vivid")

... the second call will fail. I think this is because the file-descriptor has been fully read to EOF, it is not closed/rewound, so the file-contents in the MIME for the second-call will be empty, resulting in:

{"repository":["The specified distro does not support the package you've tried to upload"]}

I'm not sure what the intended lifecycle of a Package is, but it seems reasonable to be able to reuse it. That would probably mean pulling-up from Client the file read into that class. That would also allow the File to be closed in the string/path form, which opens the file.

javabrett added a commit to javabrett/git-lfs that referenced this issue Mar 22, 2016
… re-read package File contents.

Found during work on git-lfs#1074. See also computology/packagecloud-ruby#14 .

Signed-off-by: Brett Randall <[email protected]>
@capotej
Copy link
Contributor

capotej commented Mar 22, 2016

Nice catch. I just reproduced this locally.

You are absolutely right in that a Package object is designed to be reused (this is why the distro_version_id was moved out of the Package object and into put_package recently)

I'll have a fix up for this shortly. Thanks for the report!

@capotej
Copy link
Contributor

capotej commented Mar 22, 2016

Hey @javabrett, I just released 'packagecloud-ruby 1.0.3' which should fix this issue.

Cheers!

@javabrett
Copy link
Author

Thanks for the fix!

I suppose in my mind this was between making sure that we always rewind the file, versus pulling-up the creation of all the MIME::Multipart::FormData instances from Client into Package, making the latter resource-independent.

The downside with rewinding seems to be:

  1. In the string-path version of Package constructor, we open a File which I expect we don't close anywhere.
  2. In the File version of Package constructor, we require that the File passed must remain open for the lifecycle of the Package instance.

Those might be OK. I just thought if we read the package bytes and created the MIMEs we could make Package immutable or closer to it. Should also be more efficient in the case of reusing a package file multiple times, since the file is only read and converted to MIME once.

@capotej
Copy link
Contributor

capotej commented Mar 30, 2016

Great points, I opened #16 to track. Thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants