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

hab pkg bulkupload origin creation option #7133

Merged
merged 1 commit into from
Nov 12, 2019

Conversation

jeremymv2
Copy link
Contributor

@jeremymv2 jeremymv2 commented Oct 31, 2019

Closes #7101

UX ⚙️

vagrant@chef-builder:~$ ./hab pkg bulkupload --url https://chef-builder.test --create-origins transfer/
» Preparing to upload artifacts to the 'unstable' channel on https://chef-builder.test
→ Using transfer/artifacts for artifacts and transfer/keys for signing keys.
→ Found 48 artifact(s) for upload.
☁ Discovering origin names from artifact metadata..
✓  chef
✓  core
✓  effortless
→ Found 3 origin(s) that may need to be created.
Ø Your Builder id will own any origin created. Be sure this is what you intend!
Ownership transfer is not yet implemented and will require SQL commands. [Yes/no/quit] 
...

Signed-off-by: Jeremy J. Miller jm@chef.io

Copy link
Contributor

@markan markan left a comment

Choose a reason for hiding this comment

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

A few minor nits, but otherwise looks great.

components/hab/src/cli.rs Outdated Show resolved Hide resolved
components/hab/src/main.rs Outdated Show resolved Hide resolved
components/hab/src/main.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@apriofrost apriofrost left a comment

Choose a reason for hiding this comment

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

Agree with @markan , I'd suggest making the "create origins" prompt for "yes/no/quit" part of the bulk upload flow by default so that users don't need to specify the --create-origins flag.
If users would like the missing origins created automatically without the prompt, they can use the --auto-create-origins flag to create the origins without the user confirmation.
The flow would look like this in the "future" branch:
image

@apriofrost
Copy link
Contributor

apriofrost commented Oct 31, 2019

Some nitpicks for the UX in the terminal. The goal is to keep the confirmation message consistent with what's in the GUI. Would love to hear @kagarmoe 's thoughts on it as well!

vagrant@chef-builder:~$ ./hab pkg bulkupload --url https://chef-builder.test transfer/
» Preparing to upload artifacts to the 'unstable' channel on https://chef-builder.test
→ Using transfer/artifacts for artifacts and transfer/keys for signing keys.
→ Found 48 artifact(s) for upload.
☁ Discovering origin names from artifact metadata..
✓  chef
✓  core
✓  effortless
→ Found 3 missing origin(s).
Ø Origins are required for uploading the artifacts. The Builder account that created the origin is the owner. Ownership transfer is not yet supported and will require SQL commands (link to docs if we have it?). 
Create the following origins under your Builder account?
chef
core
effortless
[Yes/no/quit] 

@jeremymv2
Copy link
Contributor Author

Some nitpicks for the UX in the terminal. The goal is to keep the confirmation message consistent with what's in the GUI.

@apriofrost I've addressed your feedback wrt utilizing --auto-create-origins in this commit

@jeremymv2 jeremymv2 requested review from markan and chefsalim November 4, 2019 21:12
Copy link
Contributor

@markan markan left a comment

Choose a reason for hiding this comment

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

The code structure suggestion is optional, feel free to take it or not.

components/hab/src/command/pkg/bulkupload.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@chefsalim chefsalim left a comment

Choose a reason for hiding this comment

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

Some minor feedback, otherwise looks great

…oad` subcommand

Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2 jeremymv2 force-pushed the jeremymv2/bulkupload_create_origins branch from d13255c to 33b7be3 Compare November 7, 2019 20:28
@jeremymv2 jeremymv2 merged commit db6f4d1 into master Nov 12, 2019
@chef-expeditor chef-expeditor bot deleted the jeremymv2/bulkupload_create_origins branch November 12, 2019 14:10
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.

hab pkg bulkupload should auto create origins if necessary
4 participants