Skip to content

Conversation

@JayZ12138
Copy link
Contributor

Pull request re-open for the one merged by mistake

Implemented basic uploading functionality, which could upload a
single file to pulp.
1. bump version and update CHANGELOG.md
2. move the upload api to FileRepository class
3. remove some unused arguments for importing to iso repo
4. upload/import file to Pulp is still blocking style, the api is just
   for import 1 file, if users want to import many files at the same time,
   should wrap it in threads
5. read file with binary mode
6. add verifications in test
various small fixes

will add example and add more test cases if the idea here's OK
@JayZ12138
Copy link
Contributor Author

about if fails the import task when repo doesn't exist in the fake client, I realized it's necessary after I left the comment and then deleted it, which caused confusion that it's in the email but can't find it in pull request anymore.

@JayZ12138 JayZ12138 changed the title 7160 Support uploading content to pulp Aug 2, 2019
@JayZ12138 JayZ12138 requested a review from rohanpm August 5, 2019 17:16
@JayZ12138 JayZ12138 requested a review from rohanpm August 6, 2019 14:10
Copy link
Contributor

@rohanpm rohanpm left a comment

Choose a reason for hiding this comment

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

The example is now working for me locally.

I think this is close to merge, but I would rather not have all the intermediate commits in history, do you want to rebase and clean up the git history yourself or should we do it at merge time?

"--password", help="Pulp password (or set PULP_PASSWORD in env)"
)
parser.add_argument("--debug", action="store_true")
parser.add_argument("--verify", default=False, action="store_true")
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please invert this so that it verifies by default?

If in doubt, copy the option name from an existing command. e.g. curl uses -k, --insecure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm, it's weird that I have already done the changes on my side and pushed, but it doesn't change in this pull request. Not sure why.

 - add verify option to upload-files
 - fix path error in upload-files
 - 'digest' should rather be 'checksum' in unit_key in import request
 - fix url error for upload
@JayZ12138
Copy link
Contributor Author

The example is now working for me locally.

I think this is close to merge, but I would rather not have all the intermediate commits in history, do you want to rebase and clean up the git history yourself or should we do it at merge time?

Yeah. I guess 'squash and merge' while doing merge will work?

@JayZ12138
Copy link
Contributor Author

Since the change in example/uploads-files didn't show, I'm closing it and reopen a new one.

@JayZ12138 JayZ12138 closed this Aug 6, 2019
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.

2 participants