dciy icon indicating copy to clipboard operation
dciy copied to clipboard

Projects can be created with blank repository URL

Open jwilm opened this issue 11 years ago • 2 comments

Visiting the new_project_path and submitting the form with an empty repository URL succeeds in creating a new project. More so, builds can be queued from this blank project, but thankfully they fail within 30 seconds when a NoMethodError is thrown.

Build output:

[DCIY] Aight, let's do this!
[DCIY] Cloning repository...
[DCIY] Looking up SHA for branch 'master'
[DCIY] SHA to build is fatal:
[DCIY] Checking out project at fatal:...
NoMethodError: undefined method `find_head' for #<Runner:0x007f887ab290d0>

/Users/jwilm/code/dciy/app/models/runner.rb:173:in `sha_for_branch'
/Users/jwilm/code/dciy/app/models/runner.rb:75:in `do_checkout'
/Users/jwilm/code/dciy/app/models/runner.rb:24:in `run_run_run'
/Users/jwilm/code/dciy/app/models/runner.rb:9:in `go_nuts_on'
/Users/jwilm/code/dciy/app/workers/build_worker.rb:13:in `perform'

...

I see two solutions with varying robustness/complexity.

  1. We can validate the repository URL is technically valid with a regex matcher. This does not guarantee the repository exists.
  2. Query the GitHub API to check that the repository exists. I guess the benefit here is that a user cannot put a typo in their URL and wonder why their builds are failing.

jwilm avatar Dec 14 '13 17:12 jwilm

Yeah, this just needs a simple validation rule in there. I don’t think it’s worth trying to query externally to make sure something exists. What I also think is worth doing is surfacing a better error if the repo/branch/sha can’t be checked out before being built (i.e. a begin/rescue block on the checkout process or something), with the error maybe suggesting that you check things like the repo name that’s been put in, the branch name is actually valid, that you have internet connectivity, and so forth.

cobyism avatar Dec 21 '13 10:12 cobyism

How about:

  1. Adding a validate :repo, presence: true in Project to catch the trivial case of "nothing there", and:
  2. Capturing stderr and stdout from the fetch and merge commands and showing them in the build output if they fail?

I think showing the underlying errors directly from git would be the easiest to diagnose. We could add a "have you checked these things" message when we detect a failure here, of course.

smashwilson avatar Dec 21 '13 16:12 smashwilson