Project

General

Profile

Actions

Bug #17530

closed

[arvados-client] when ARVADOS_API_HOST/TOKEN are not set, error out quickly

Added by Ward Vandewege over 3 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Story points:
-
Release relationship:
Auto

Description

arvados-client then tries to connect to an empyt ARVADOS_API_HOST and times out after 30s.

What it should do is error out immediately with an appropriate message.


Subtasks 1 (0 open1 closed)

Task #17538: Review 17530-arvados-client-fastfail-take2ResolvedNico César04/15/2021Actions
Actions #1

Updated by Ward Vandewege over 3 years ago

  • Description updated (diff)
Actions #2

Updated by Lucas Di Pentima over 3 years ago

  • Assigned To set to Lucas Di Pentima
Actions #3

Updated by Lucas Di Pentima over 3 years ago

  • Assigned To changed from Lucas Di Pentima to Nico César
Actions #4

Updated by Nico César over 3 years ago

  • Status changed from New to In Progress

Is this a current ticket? I compiled arvadsos-client from e46caaf835e32106e2da5aa7f895435bd4718da6

I tried with

unset ARVADOS_API_HOST
unset ARVADOS_API_TOKEN
./arvados-client get zzzzz-somej-12345678901234
ARVADOS_API_HOST and/or ARVADOS_API_TOKEN environment variables are not set

and

export ARVADOS_API_TOKEN="" 
export ARVADOS_API_HOST="" 
./arvados-client get zzzzz-somej-12345678901234
ARVADOS_API_HOST and/or ARVADOS_API_TOKEN environment variables are not set

both of them failed fast

Actions #5

Updated by Nico César over 3 years ago

  • Assigned To changed from Nico César to Ward Vandewege
  • Status changed from In Progress to Feedback

@Ward Vandewege, Can you describe how to reproduce this error?

Actions #6

Updated by Ward Vandewege over 3 years ago

Nico César wrote:

@Ward Vandewege, Can you describe how to reproduce this error?

Maybe it's specific to certain subcommands, e.g. (from current head):

$ ./arvados-client costanalyzer ce8i5-xvhdp-83iel786l8dkxv4
2021/04/14 15:57:14 error reading "/etc/arvados/ca-certificates.crt": open /etc/arvados/ca-certificates.crt: permission denied
2021/04/14 15:57:26 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:57:35 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:57:44 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:57:53 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:58:02 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
2021/04/14 15:58:11 WARNING: Error retrieving services list: Get "https:///arvados/v1/keep_services/accessible": http: no Host in request URL (retrying in 3s)
error creating Keep object: timed out while getting initial list of keep services

Maybe it only happens when the keep Go sdk is used?

Actions #7

Updated by Nico César over 3 years ago

Commit:841a23635 17530-arvados-client-fastfail

developer-run-tests: #2415

Actions #8

Updated by Nico César over 3 years ago

d6c8a79b073cb000c7914e3ef07bf4d95350ae4f 17530-arvados-client-fastfail-take2

Actions #9

Updated by Ward Vandewege over 3 years ago

Additional fix in e758989ebd80f3365baa56be2079d72e286c9cf5 on branch 17530-arvados-client-fastfail-take2, ready for a look.

Actions #10

Updated by Ward Vandewege over 3 years ago

  • Assigned To changed from Ward Vandewege to Nico César
Actions #11

Updated by Tom Clegg over 3 years ago

I think it would be better to make (*arvadosclient.ArvadosClient)Call() fail fast on an empty host. That will cover a lot more cases, and then the extra fail-fast in keepclient.New() will be superfluous.

Also, keep in mind it's not an error to get the discovery doc or keep_services with an empty token. We should only be checking for blank host here. Let the server decide whether "no token" is an error.

Another bit of feedback: "API server is not configured" seems like a misdirection, since it's the client we're talking about, not the server... how about "Arvados client is not configured (target API host is not set). Maybe env var [...]" or something like that?

Actions #12

Updated by Tom Clegg over 3 years ago

Sorry, I think I was unclear. When I said "better to make ... Call()" I meant "...better than the extra fail-fast in keepclient.New()" (see note 9).

We still need the fail-fast in discoverServices().

So in summary, I think we need a total of two changes:
  1. discoverServices() should return an error before adding an entry to svcListCache, starting a poll loop, etc. if host is empty
  2. CallRaw in arvadosclient should return an error instead of trying http calls if host is empty
Actions #13

Updated by Nico César over 3 years ago

Squashed at 1f1c71924 17530-arvados-client-fastfail-take2

developer-run-tests: #2426

Actions #14

Updated by Tom Clegg over 3 years ago

Other than the commit message being a little confusing (it seems to use future tense to describe the old behavior of CallRaw() which is no longer true)... LGTM, thanks!

Actions #15

Updated by Anonymous over 3 years ago

  • % Done changed from 0 to 100
  • Status changed from Feedback to Resolved
Actions #16

Updated by Peter Amstutz over 3 years ago

  • Release set to 38
Actions

Also available in: Atom PDF