Bug #13892
closed[arvados-cwl-runner] Include a deprecation warning if using jobs API
Added by Nico César over 6 years ago. Updated over 6 years ago.
100%
Description
As user of CWL on Arvados, I would like to be informed of usage of deprecated APIs, such as the Jobs API.
A web link to a more extensive explanation such as https://dev.arvados.org/projects/arvados/wiki/Containers_API which could be enhanced over time to provide the most current information would be useful.
Updated by Tom Morris over 6 years ago
- Subject changed from [CWL] [cwl-runner] Include a deprecation warning if using jobs API to [arvados-cwl-runner] Include a deprecation warning if using jobs API
- Description updated (diff)
- Target version set to To Be Groomed
Updated by Tom Morris over 6 years ago
- Target version changed from To Be Groomed to Arvados Future Sprints
- Story points set to 0.5
Updated by Peter Amstutz over 6 years ago
- Status changed from New to In Progress
- Target version changed from Arvados Future Sprints to 2018-08-01 Sprint
13892-deprecate-jobs-api @ c7cbb121dec3c356bf7d72087d601c7593410b8c
Updated by Nico César over 6 years ago
review at c7cbb121dec3c356bf7d72087d601c7593410b8c
I see that disable_api_methods: ["jobs.create", "pipeline_instances.create"] is short compared what we've been doing #10367#note-10. Should we include more?
From my perspective, I'd like customer to be aware that "disabling all api jobs"(like in 10367) versus "making the cluster unable to accept jobs", since users will be questioning "why is this 404/422/500 error showing up, and I can see I did this before". What I mean here: is the message oriented for the sysadmin or for the user, and if it's the later, it should be clear if the cluster is able to do the task or not, and what is the documentation for helping the user.
Updated by Peter Amstutz over 6 years ago
Nico César wrote:
review at c7cbb121dec3c356bf7d72087d601c7593410b8c
I see that disable_api_methods: ["jobs.create", "pipeline_instances.create"] is short compared what we've been doing #10367#note-10. Should we include more?
From my perspective, I'd like customer to be aware that "disabling all api jobs"(like in 10367) versus "making the cluster unable to accept jobs", since users will be questioning "why is this 404/422/500 error showing up, and I can see I did this before". What I mean here: is the message oriented for the sysadmin or for the user, and if it's the later, it should be clear if the cluster is able to do the task or not, and what is the documentation for helping the user.
Those two methods are the ones which clients (a-c-r, workbench) use to probe for whether the jobs/pipelines API is available or not. So disabling those is the minimum required to alter client behavior.
The message is a little bit tricky. The assumption is that what the user wants is to get rid of the warning and use the containers API by default. However, currently we only support that by disabling the jobs API entirely.
So another approach would be to introduce a user preferences file, so the user could do something like "arvados-cwl-runner --set-default-api=containers" and then have it remember that.
Updated by Nico César over 6 years ago
Peter Amstutz wrote:
Nico César wrote:
review at c7cbb121dec3c356bf7d72087d601c7593410b8c
I see that disable_api_methods: ["jobs.create", "pipeline_instances.create"] is short compared what we've been doing #10367#note-10. Should we include more?
From my perspective, I'd like customer to be aware that "disabling all api jobs"(like in 10367) versus "making the cluster unable to accept jobs", since users will be questioning "why is this 404/422/500 error showing up, and I can see I did this before". What I mean here: is the message oriented for the sysadmin or for the user, and if it's the later, it should be clear if the cluster is able to do the task or not, and what is the documentation for helping the user.
Those two methods are the ones which clients (a-c-r, workbench) use to probe for whether the jobs/pipelines API is available or not. So disabling those is the minimum required to alter client behavior.
Is that 100% true on workbench? the reason I brought up this is because the customer kept finding ways that workbench was failing. if you see #10367 the list was growing over time. because of this reason
The message is a little bit tricky. The assumption is that what the user wants is to get rid of the warning and use the containers API by default. However, currently we only support that by disabling the jobs API entirely.
I see, but "disabling the jobs api" is an instruction for the sysadmin. Once done, pipelines will fail to the user, without any understanding why. Just like something like 404 or MethodNotFound.
It will be good to say "hey, user!, we're deprecating this feature still supported in your cluster, here <URL FOR USERS> is a guide on how to migrate to containers API, besides you can try it out with --api=containers right away. Hey sysadmin, when you're ready to deprecate follow this guide <URL FOR SYSADMIN>". And once is disabled, "hey, user!, the feature you are requesting has been deprecated, here <URL FOR USERS> is a guide on how to migrate to containers API".
Does it make sense? maybe I'm overdoing the messages, but it seems to me that helps a LOT the communication with users, since the cluster parameters are opaque to them, they only see what the tools report.
So another approach would be to introduce a user preferences file, so the user could do something like "arvados-cwl-runner --set-default-api=containers" and then have it remember that.
Is this assuming a stateful arvados-cwl-runner running from a shell node? remember that some of our runs are from automatic pipelines and some on sporadic VMs that only have the API token. I don't think this approach is the right one, but I could be wrong.
Updated by Nico César over 6 years ago
review at e8ebb2101fa89294c91b404953cc2fbabb796e40
Looks good to me. Ready to merge.
Updated by Peter Amstutz over 6 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|b727cb3c8c32389f03e6ae71ad128c518a2c2a8c.