Project

General

Profile

Actions

Bug #5688

closed

[API] Accept a git revision range in runtime_constraints, deprecate "min" and "max" revision filters.

Added by Tom Clegg about 9 years ago. Updated almost 8 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
-
Category:
API
Target version:
-
Story points:
2.0

Description

The current set of miscellaneous git revision filters makes it possible to select versions, but it is hard to understand because it doesn't correspond to git terminology or the kinds of filtering done by other git tools.
  • For example, a client can specify a start and end commit ("min" and "max") but unlike git, such a range is defined to include the "min" commit, and is defined to consist of the two commits "min" and "max" (instead of the empty set) when there is no path from min to max.

Additionally, the current interaction between "script_version" and [the various ways of specifying] "minimum_script_version" is mysterious: script_version is interpreted as either a specific commit or one end of a range of commits, depending on context.

Additionally, the current interaction between "script_version" and "supplied_script_version" attributes is mysterious, partly because the client has to specifically request that supplied_script_version be updated:
  • create job with script_version=branchA → job has script_version=sha1A, supplied_script_version=branchA
  • update job with script_version=branchB → job has script_version=sha1B, supplied_script_version=branchA (misleading)
  • update job with supplied_script_version=branchX → job has script_version=sha1B, supplied_script_version=branchX (misleading)
  • update job with supplied_script_version=nil → job has script_version=sha1B, supplied_script_version=nil (mysterious)
  • update job with repository=foobar → job has script_version=sha1B, supplied_script_version=sha1B (useless)
  • update job with script_version=branchA → job has script_version=sha1A, supplied_script_version=sha1B (misleading but at least obviously wrong)
This could be addressed (without loss of generality) with an API like this:
  • script_version is always saved as a full sha1. If submitted as some other gitrevision that resolves to a commit (typically a branch name), that commit's full sha1 is saved instead. If it can't be resolved to a commit, the create/update transaction fails. (This is what we had before supplied_script_version was introduced.) This lookup-and-replace feature is deprecated, though, and the following feature should be used instead, with script_version=nil:
  • If the runtime_constraints hash has a script_version key, the value given there is a git revision range as described in gitrevisions(1).
    • If no commits are in the given range, the transaction fails.
    • If script_version is not nil and is not in the given range, the transaction fails.
    • If script_version is not nil and is in the given range, it is resolved to a sha1 and saved.
    • If script_version is nil and any commits are in the given range, a script_version is assigned a commit sha1 from the given range. (In a create transaction, find_or_create uses this range to decide whether an existing job can be reused.)
  • script_version is left alone during an update transaction, and no attempt is made to resolve or validate any gitrevisions, if none of {repository, script_version, runtime_constraints[script_version]} are being changed.
  • The only way script_version is edited by the API server is to change nil to a sha1 — or to resolve a named revision to a sha1, in the deprecated backward-compatibility case.
  • repository and runtime_constraints are never edited by the API server.

This means the correct way for clients (like Workbench and arv-run-pipeline-instance) to submit jobs will be to put the requested version in runtime_constraints[script_version], instead of script_version as they do now. This way, Workbench can offer the "re-run job using current branchA? or same sha1 used for original?" feature without the confusing API and loopholes that allow supplied_script_version to go out of sync.

This same pattern is already used for arvados_sdk_version and docker_image_locator. The code for arvados_sdk_version should be updated to accommodate gitrevisions.

The supplied_script_version database column should be deleted. Temporarily, for backward compatibility, include a copy of runtime_constraints[script_version] as supplied_script_version in API responses.


Related issues

Related to Arvados - Bug #5490: [Crunch] Job should not be reused when docker tag _or_ arvados sdk version now resolves to a different image/commitResolvedBrett Smith04/21/2015Actions
Actions #1

Updated by Tom Clegg about 9 years ago

  • Description updated (diff)
  • Category set to API
Actions #2

Updated by Tom Clegg about 9 years ago

  • Subject changed from [API] Accept a git revision range as "in git" filter operand, deprecate "min" and "max" revision filters. to [API] Accept a git revision range in runtime_constraints, deprecate "min" and "max" revision filters.
  • Description updated (diff)
  • Story points changed from 1.0 to 2.0
Actions #3

Updated by Brett Smith almost 9 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:03d89302f14a4a1c448b32d62679bab14e23fa23.

Actions #4

Updated by Brett Smith almost 8 years ago

  • Target version deleted (Arvados Future Sprints)
Actions

Also available in: Atom PDF