Add repo validation, enable branch selection

[oliverroick]

Contributes to #66: Extends the build-image form, so users can select a repository and branch, and add stronger validation for inputs.

Changes

• Adds a "Repository" field, which takes the repository for the image build.
  • It can be provided either as org/repo or github.com/org/repo or https://github.com/org/repo. Other inputs should be rejected with an error.
  • The entered repo is further validated against the Github API. We're sending a HEAD request to verify if the repository exists and if it is public.
• Adds a "Branch" field, allowing users to select the branch to build from.
  • The branch field will only be shown after the repository input has been verified.
  • We're using the GitHub API to request a list of available branches, the select field is populated from the list of branches.
• Adds a placeholder for "Provider", at the moment this only says "GitHub"

[batpad]

@oliverroick this looks really good to me and seems to work well!

I guess my only question is that since we will plan on adding more Providers down the line, do you think it might add too much complexity to do the repo validation and branch name fetching for each one?

The branch-picker drop-down is nice - I wonder if users might ever want to just put a Git Sha or so instead of a branch name? Since I think currently that accepts anything that's a valid ref. Getting that drop-down is nice and convenient so it might be nice to leave in for now, but just a heads-up that we may want to support refs that are not branch names (like commit hashes)

[batpad]

Did you mean to remove this @oliverroick ?

[oliverroick]

Weird, I thought this wasn't part of the dependencies, but it looks like it is. I'll revert the change.

[batpad]

Noticed a couple things:

• If I paste in the full URL like https://github.com/NASA-IMPACT/pangeo-notebook-veda-image/, I think it should "normalize" the value in the Repository field to be like NASA-IMPACT/pangeo-notebook-veda-image. Or else, we will need to make sure to send the right "org/repo" when we Build. @oliverroick happy to talk through this quickly - I'm not sure what the best behaviour here should be.
• On the workflow to select branches from the Github API: I realized we'd also want users to be enter Git tags, and at that point I wonder if it's easier to just leave it as an optional text input?

[oliverroick]

I guess my only question is that since we will plan on adding more Providers down the line, do you think it might add too much complexity to do the repo validation and branch name fetching for each one?

I don’t think so. I envisage this as separate components for each provider, which encapsulates the form-field generation and validation, with separate hook implementations for each of those. There will be some repetition but I think it’s fine because they are all different concepts.

If I paste in the full URL like https://github.com/NASA-IMPACT/pangeo-notebook-veda-image/, I think it should "normalize" the value in the Repository field to be like NASA-IMPACT/pangeo-notebook-veda-image. Or else, we will need to make sure to send the right "org/repo" when we Build. @oliverroick happy to talk through this quickly - I'm not sure what the best behaviour here should be.

I thought about doing that a fair bit but decided against it. I usually try to avoid changing user input because there could be weird consequences. Like when do you change the value? When they stop typing, when the repository is validated? I left it where it is as to not introduce more complexity to the user interface. We’re doing the normalisation in the background. The normalised value is stored in repoId, which is what we’re using the start the build process.

On the workflow to select branches from the Github API: I realized we'd also want users to be enter Git tags, and at that point I wonder if it's easier to just leave it as an optional text input?

Let me have a look into that. We can also get the tags and compose a list of both branches and tags. There’s no major difference, just one more HTTP request to the GitHub API.

The branch-picker drop-down is nice - I wonder if users might ever want to just put a Git Sha or so instead of a branch name?

If we want to support Git SHAs too, then I’d revert to a text field. We could add validation, and check the GitHub API if the branch/tag/sha exists. Should we do that or do the branch/tag approach for now?

[batpad]

If we want to support Git SHAs too, then I’d revert to a text field. We could add validation, and check the GitHub API if the branch/tag/sha exists. Should we do that or do the branch/tag approach for now?

Let's do branch / tag for now!

Like when do you change the value? When they stop typing, when the repository is validated? I left it where it is as to not introduce more complexity to the user interface. We’re doing the normalisation in the background. The normalised value is stored in repoId, which is what we’re using the start the build process.

Sounds good, thank you for thinking this through - I agree.

From testing locally, it seemed like it was trying to use the full http:// URL for the build and not the normalised value - i.e. the build did not work when I put in the full repo URL but did work when I just had org/repo. Maybe just a silly bug somewhere?

[batpad]

Thanks for the fixes @oliverroick ! I think I feel okay about merging this - shall I go ahead?

[oliverroick]

Not yet @batpad, still need to build the tag select

[oliverroick]

@batpad I've added the tag selection, can you give this another spin?

[batpad]

@batpad I've added the tag selection, can you give this another spin?

Looks good to me!

Very minor quibble:

If I say, paste the repository URL into the repository field and then without leaving focus from the field, click the "Build Image" button, the behaviour is slightly strange - i.e. it will then show the branch field. I think this is okay. The other option I can think of, that may make this slightly better: always render the Branch / Tag field, but keep it disabled until the user has entered the repository field, so that it's clear that there is one more field to fill up before one can submit.

Am fine with merging as is though - would leave this to be your call @oliverroick . Thanks again for this, it's looking really good!

[batpad]

@oliverroick the updates look really good to me.