Story #9551

[SDKs] Python SDK supports an ARVADOS_KEEP_SERVICES environment variable

Added by Brett Smith almost 5 years ago. Updated almost 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
07/06/2016
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Description

ARVADOS_KEEP_SERVICES is an environment variable that lets the client set its own list of Keep services, instead of fetching it from the API server. It is a space-separated list of Keep API endpoint URLs. Those endpoints are expected to be non-disk services: the client should assume it does not know how many replicas the service will store, should send the X-Keep-Desired-Replicas request header, and respect the X-Keep-Replicas-Stored response header.

A client should pay attention to this env var if its api_host/token/insecure settings were also taken from the environment. IOW, treat it as an additional client config entry, rather than checking the environment variable from the depths of KeepClient.

Functional requirements:

  • When the variable is set, it does not query the API server for the Keep services list, or respect the ARVADOS_KEEP_PROXY environment variable. Instead it uses the value of the variable as the services list, following the rules above.
  • When the variable is unset or empty, the Keep client should behave as it currently does: if ARVADOS_KEEP_PROXY is set, it treats that as a single service to use. Otherwise, it gets the list of Keep services from the API server.

Subtasks

Task #9649: Review 9551-python-keep-services-envResolvedTom Clegg


Related issues

Copied from Arvados - Story #9550: [SDKs] Go SDK supports an ARVADOS_KEEP_SERVICES environment variableResolved07/06/2016

Associated revisions

Revision e31f6c9e
Added by Lucas Di Pentima almost 5 years ago

Merge branch '9551-python-keep-services-env'

9551: Python SDK support for ARVADOS_KEEP_SERVICES environment variable. Closes #9551

History

#1 Updated by Tom Clegg almost 5 years ago

  • Description updated (diff)

#2 Updated by Brett Smith almost 5 years ago

  • Target version set to Arvados Future Sprints

#3 Updated by Tom Clegg almost 5 years ago

  • Description updated (diff)

#4 Updated by Brett Smith almost 5 years ago

  • Story points set to 1.0

#5 Updated by Tom Clegg almost 5 years ago

  • Assigned To set to Tom Clegg
  • Target version changed from Arvados Future Sprints to 2016-08-03 sprint

#6 Updated by Lucas Di Pentima almost 5 years ago

  • Assigned To changed from Tom Clegg to Lucas Di Pentima

#7 Updated by Lucas Di Pentima almost 5 years ago

  • Status changed from New to In Progress

#8 Updated by Lucas Di Pentima almost 5 years ago

2aefffe61af4bb93659ca1cd9a9a027858658bec

Added support of ARVADOS_KEEP_SERVICES env var by enhancing the code that takes care of setting up a proxy keep service.

#9 Updated by Tom Clegg almost 5 years ago

It looks like the new precedence order is
  1. ARVADOS_KEEP_SERVICES
  2. "proxy" kwarg
  3. ARVADOS_KEEP_PROXY

The docstring claims that proxy='' prevents KeepClient from using ARVADOS_KEEP_PROXY, so we should probably make it override ARVADOS_KEEP_SERVICES too.

Should also update that docstring to mention the new env var.

Should update the use of ARVADOS_KEEP_PROXY in the test suite to use the new env var instead.

Nit: proxy_uris.index(uri) seems like an inefficient way to get the loop index. Also, if the list has two identical entries, we'll have two identical UUIDs, which could cause strangeness elsewhere. How about doing an idx++ each time through the loop? (Or some other more Pythonic equivalent of Go's "for i, x := range a"?)

Nit: In the test case, might as well use examples with realistic URI formats, like http://10.1.0.1 and https://bar.example.com:12345/

#10 Updated by Lucas Di Pentima almost 5 years ago

Tom Clegg wrote:

It looks like the new precedence order is
  1. ARVADOS_KEEP_SERVICES
  2. "proxy" kwarg
  3. ARVADOS_KEEP_PROXY
    The docstring claims that proxy='' prevents KeepClient from using ARVADOS_KEEP_PROXY, so we should probably make it override ARVADOS_KEEP_SERVICES too.

Corrected. Now the proxy param has the priority again, and ARVADOS_KEEP_SERVICES is second.

Should also update that docstring to mention the new env var.

Updated.

Should update the use of ARVADOS_KEEP_PROXY in the test suite to use the new env var instead.

Done!

Nit: proxy_uris.index(uri) seems like an inefficient way to get the loop index. Also, if the list has two identical entries, we'll have two identical UUIDs, which could cause strangeness elsewhere. How about doing an idx++ each time through the loop? (Or some other more Pythonic equivalent of Go's "for i, x := range a"?)

Corrected. enumerate() is the function that I couldn't find before and is the mentioned Go equivalent.

Nit: In the test case, might as well use examples with realistic URI formats, like http://10.1.0.1 and https://bar.example.com:12345/

Corrected.

Also, added URL validation using urlparse module.

Thanks!

6f6590fc776e42b3da8e09fbcda0b47c3c991227

#11 Updated by Tom Clegg almost 5 years ago

Thanks! 6f6590f LGTM

#12 Updated by Lucas Di Pentima almost 5 years ago

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

Applied in changeset arvados|commit:e31f6c9eb427302eb48150d431afcc962954b061.

Also available in: Atom PDF