Bug #6950

[Documentation] Reconsider job creation test in Crunch dispatcher install guide

Added by Brett Smith about 4 years ago. Updated about 4 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
-
Category:
Documentation
Target version:
Start date:
08/11/2015
Due date:
% Done:

100%

Estimated time:
(Total: 1.00 h)
Story points:
0.5

Description

As of this writing, the Crunch dispatch install guide includes a "Git Repositories" section that includes a sample job you can submit to make sure that the API server can read your repositories. Its location is kind of funny: it's written before we tell you to run crunch-dispatch, so it's really testing the prerequisites of the API server and its access to Git repositories. And it would still be that even if we moved it to the bottom of the page, because at this point in the install guide, you haven't installed a compute node, so crunch-dispatch can't dispatch anything.

Now that we've done #6663, this might be better moved to the bottom of the git server install guide. It can serve as a check that repository configuration and permissions work for both the API server and Gitolite.


Subtasks

Task #6960: Review 6950-install-guide-job-test-wipResolvedBrett Smith

Associated revisions

Revision 3401991d
Added by Brett Smith about 4 years ago

Merge branch '6950-install-guide-job-test-wip'

Closes #6950, #6960.

History

#1 Updated by Tom Clegg about 4 years ago

I agree this is a non sequitur in its current place, and it would make more sense at the end of git setup page, at the first point where it's expected to work and failure suggests "stay on this page and check your work", not "go back to the previous page and check your work."

I think it would be easy + helpful to make two more changes:
  • Mention that the job won't actually run yet
  • Use an abbreviated commit hash, and check the API response to confirm it was resolved to the full hash. Might be a little more convincing than just "didn't get this error". ("master" will be resolved to a full hash too, but it might be less smooth to explain what you should see in the response. "You should see 522404b2066c8b635a240f79dd4bf652479afadb" vs. "You should see the same commit hash that you see at the top of https://github.com/curoverse/arvados/commits/master.")

#2 Updated by Brett Smith about 4 years ago

  • Status changed from New to In Progress
  • Assigned To set to Brett Smith
  • Target version changed from 2015-09-02 sprint to 2015-08-19 sprint

Taking this on because I already half-wrote the patch so I might as well finish the job.

#3 Updated by Brett Smith about 4 years ago

  • Status changed from In Progress to New
  • Assigned To deleted (Brett Smith)
  • Target version changed from 2015-08-19 sprint to 2015-09-02 sprint

#4 Updated by Brett Smith about 4 years ago

Tom Clegg wrote:

I agree this is a non sequitur in its current place, and it would make more sense at the end of git setup page, at the first point where it's expected to work and failure suggests "stay on this page and check your work", not "go back to the previous page and check your work."

As the guide is currently written, it's not runnable there, because no repositories exist in the system. That doesn't happen until "Create standard objects," which is a little later, because you also need a shell node to create those objects.

Running it at the end of "Create standard objects" is the soonest we could do it. It feels a little out of place there, though.

We could move it to the top of the Crunch dispatch install guide, as a way to check that all the prerequisite components are set up correctly before proceeding.

I think it would be easy + helpful to make two more changes:
  • Mention that the job won't actually run yet
  • Use an abbreviated commit hash, and check the API response to confirm it was resolved to the full hash. Might be a little more convincing than just "didn't get this error". ("master" will be resolved to a full hash too, but it might be less smooth to explain what you should see in the response. "You should see 522404b2066c8b635a240f79dd4bf652479afadb" vs. "You should see the same commit hash that you see at the top of https://github.com/curoverse/arvados/commits/master.")

Both of these sound good to me.

#5 Updated by Tom Clegg about 4 years ago

Brett Smith wrote:

As the guide is currently written, it's not runnable there, because no repositories exist in the system. That doesn't happen until "Create standard objects," which is a little later, because you also need a shell node to create those objects.

Running it at the end of "Create standard objects" is the soonest we could do it. It feels a little out of place there, though.

Agree.

We could move it to the top of the Crunch dispatch install guide, as a way to check that all the prerequisite components are set up correctly before proceeding.

I like this idea: "If everything has gone well so far, we can create a job: [...] Now we can see it's queued, waiting for us to set up the infrastructure to run it... so let's do that."

#6 Updated by Brett Smith about 4 years ago

  • Status changed from New to In Progress
  • Target version changed from 2015-09-02 sprint to 2015-08-19 sprint

I've done the above in 6950-install-guide-job-test-wip.

#7 Updated by Tom Clegg about 4 years ago

LGTM -- just two nits:

"your collection's content address" seems a bit mysterious -- afaics we haven't created a collection in the install guide yet, and it might not be obvious enough that we use "portable data hash" and "content address" interchangeably. Is it worth putting "echo -n |arv-put --portable-data-hash -" in front of this, to make it concrete? The output of that command should always be 4c62f0a0d2344608e5f197894beb6fb5+47 so we wouldn't even need a "put yours here" part.

"either because it doesn't exist, or the user running the API server doesn't" - seems like it should be "because either A or B" or "either because A or because B" or even just "either A or B".

#8 Updated by Brett Smith about 4 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:3401991df49ba927ac4b1fe47c6e3b7ad1d26883.

Also available in: Atom PDF