Arvados

Currently, arvados-cwl-runner defaults to the crunch v1 jobs API. This means sites using crunch v2 need to explicitly add --api=containers.

arvados-cwl-runner should be able to discover from the discovery document whether to prefer the jobs or containers API.

10291-discovery-blacklist

test 6bf3b279cd2d5ea5c1d6b849265b7206461467b8

Reviewing at 6bf3b27

• Can you please include an example with multiple methods in the config yml (and expand the test to check)?
  

    disable_api_methods: []
    Example: ["jobs.create"]
  

• init.py: It appears that “jobs” would be used if no “api” is specified? Do we change the order in “for api in ["jobs", "containers”]” so that “containers” is checked first?
  • BTW, if I were to do this, only 3 tests seem to get unhappy

• Do we have any tests with "containers" api used? (On looking at the diff, I felt they all were using jobs api)

Radhika Chippada wrote:

• Can you please include an example with multiple methods in the config yml (and expand the test to check)?

Added

• init.py: It appears that “jobs” would be used if no “api” is specified? Do we change the order in “for api in ["jobs", "containers”]” so that “containers” is checked first?
  • BTW, if I were to do this, only 3 tests seem to get unhappy

At least for now, we still want to use jobs by default if both APIs are available.

• Do we have any tests with "containers" api used? (On looking at the diff, I felt they all were using jobs api)

That sounds like a good idea but I think it's out of scope here. tests.test_job.TestWorkflow.test_default_work_api tests the added behavior (i.e., the default changes to "containers" if "jobs" is disabled).

Updated API to return 404 for disabled APIs → 10291-really-disable-apis, test cfa2c529c7c8105e4d424b404db2c48ff2066e27

Updated API to return 404 for disabled APIs

That's better. LGTM

In thinking about the wide ranging impact of obsoleting a core API and introducing an incompatible API to replace it, I'm wondering if this is solving the wrong problem.

How much easier would it make our life if we just committed to making the new API backward compatible with the old? Is there a reason that wouldn't be the best solution to this problem?

Tom Morris wrote:

In thinking about the wide ranging impact of obsoleting a core API and introducing an incompatible API to replace it, I'm wondering if this is solving the wrong problem.

How much easier would it make our life if we just committed to making the new API backward compatible with the old? Is there a reason that wouldn't be the best solution to this problem?

The semantics of Crunch v2 are significantly different from Crunch v1 and they have non-overlapping behaviors such that neither one is a subset of the other. We could reimplement the crunch v1 'jobs' API using the crunch v2 API in order to support existing crunch scripts, but this requires filling in for crunch v1 behavior that doesn't exist in crunch v2, which is more work, not less.

We are using CWL as the bridge from crunch v1 to crunch v2. The branch in this ticket is in line with this strategy, by making arvados-cwl-runner automatically determine the appropriate API to submit to.

A new implementation of the crunch1 API using crunch2 under the hood is surely a good feature.

However, that isn't expected to happen right away, and in the meantime it seems better if the API server doesn't offer to do things it can't do...