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

Check name #32

Merged
merged 2 commits into from
Jun 10, 2019
Merged

Check name #32

merged 2 commits into from
Jun 10, 2019

Conversation

pmreyes2
Copy link
Contributor

checks if creating the bucket with the user's given input is possible before proceeding.

@pmreyes2 pmreyes2 requested a review from ljlamarche June 10, 2019 21:20
@ljlamarche
Copy link
Contributor

Looks good! Merging.

@ljlamarche ljlamarche merged commit 13139aa into enh_guided_create_bucket Jun 10, 2019
@ljlamarche ljlamarche deleted the check_name branch June 10, 2019 21:25
@asreimer
Copy link
Contributor

This isn't what we wanted is it? I thought we talked about the following:

  1. Ask a user for a bucket name
  2. Is bucket name valid? Yes, go to 3. No, error message and go back to 1.
  3. Ask user for next thing
  4. Is next thing valid? Yes, got to 5. No, error message and go back to 3.

This PR doesn't do that at all.

@ljlamarche
Copy link
Contributor

I'm still working on validating the rest of the inputs. I'll push commits directly to enh_guided_create_bucket. From my perspective, the following happens when name_test already exists:

[resen] >>> create_bucket name_test
Creating bucket with name: name_test
ERROR: Bucket with name: name_test already exists!
Failed to create bucket!

And then it returns you to the [resen] >>> prompt without creating a bucket. This seems reasonable to me.

Having to specify the name as an input with the rest would work too, so I guess it's just user preference?
Currently, workflow is:

[resen] >>> create_bucket input
Creating bucket with name: input
Please choose a version of resen-core. Available versions: 2019.1.0rc1
>>> Select a version: 2019.1.0rc1
... ect

Your proposal is that we instead do:

[resen] >>> create_bucket
Provide a name for the bucket. Names must be less than 20 characters long and start with an alphabetic character.
>>> Bucket name: input
Please choose a version of resen-core. Available versions: 2019.1.0rc1
>>> Select a version: 2019.1.0rc1
... ect

Personally, I'm perfectly happy with either way.

@asreimer
Copy link
Contributor

Erroring out back to the resen shell prompt is an entirely different process. For example, do we now want to error out of the bucket creating if a specified mount location isn't valid? If we crash out for an invalid bucket name, why do we do something different for storage volumes? Or even more generally, why don't we just crash back for invalid input at any point?

Whatever we do, we need to be consistent. Also, it's hard to work on a project when we discuss one thing but then implement something different right after the discussion.

@ljlamarche
Copy link
Contributor

Ok, from the user perspective, erroring out back to the resen shell prompt after entering an invalid name is fine, but erroring out if an invalid path is entered 3 steps later would be annoying. If we're going to be consistent and treat all inputs the same, we probably want to go for the second approach (sorry, I think I misunderstood the distinction in our discussions). I'm putting together validation functions for all the inputs now, roughly following the model of get_valid_input.

docker_image = self.get_valid_input(msg,valid_versions)

This should help manage this consistently with all inputs.

@asreimer
Copy link
Contributor

That's true, that would be quite annoying. From a user perspective, it might be nicer if the intent is to actually create a bucket then for the create_bucket function to not give up immediately just because the user entered an invalid name. It just tells the user and they can try again, instead of having to type create_bucket again.

I think what you wrote before, in your last example where create_bucket takes no inputs, is the best way to do things.

@ljlamarche
Copy link
Contributor

Ok, sounds good. Here's an example of the new workflow.

[resen] >>> create_bucket
Please enter a name for your bucket.  Valid names must be less than 20 characters and start with an aphabetic character
>>> Enter bucket name: name_test
ERROR: Bucket with name: name_test already exists!
Cannot use the same name as an existing bucket!
>>> Enter bucket name: name test
Bucket names may not contain spaces.
>>> Enter bucket name: 0name
Bucket names must start with alphabetic characters.
>>> Enter bucket name: thisisanextremelylongnameforabucket
Bucket names must be less than 20 characters.
>>> Enter bucket name: name_test2
Please choose a version of resen-core. Available versions: 2019.1.0rc1

(Please ignore the spelling of "alphabetic".) If this looks good, I'll write functions that handle the other input in a similar fashion.

@asreimer
Copy link
Contributor

This looks good to me.

I think that we (and especially me) need to write out examples like you just did here when we have discussions about what we want to do. This makes it explicitly clear what we want to do. @pmreyes2 has asked for that in the past and I completely forgot until now.

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