Project

General

Profile

Actions

Idea #20613

closed

Reveal googleapiclient retry logs during client construction

Added by Brett Smith over 1 year ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
SDKs
Start date:
06/07/2023
Due date:
Story points:
1.0
Release relationship:
Auto

Description

Until we have a totally sorted story for googleapiclient logs (#20521), we at least want users to be able to see retry logs while the client is being created, so if they, e.g., have a typo in their ARVADOS_API_HOST, they'll see a message about that ASAP.

  • Add a APIClientLogFilter to the Python SDK that emits retry messages at the info level. (The details of this filter may change later in #20521, but the API will remain the same.)
  • Update arvados.api constructor(s) to make sure these logs go somewhere on at least the first client instantiation.
    • Exact strategy TBD depending on whether logging objects are thread-safe, etc. It is acceptable if these logs only appear on the very first instantiation, but that limitation is not required either.
    • Should at least install the filter and ensure googleapiclient logs have some handler, maybe carry one over from arvados if not.
  • Remove the googleapiclient level-setting stuff from a-c-r.
  • Ensure googleapiclient logs appear during a-c-r's client instantiation.
    • The handler copy idea above might be sufficient. If not, this might need input from Peter about the best strategy.

Subtasks 1 (1 open0 closed)

Task #20615: Review 20613-googleapiclient-init-logsIn ProgressPeter Amstutz06/07/2023Actions

Related issues 2 (1 open1 closed)

Related to Arvados - Idea #20521: Python tools emit logs from googleapiclientNewActions
Related to Arvados - Bug #20611: Creating api object hangs when inside crunch containerResolvedPeter AmstutzActions
Actions #1

Updated by Brett Smith over 1 year ago

  • Related to Idea #20521: Python tools emit logs from googleapiclient added
Actions #2

Updated by Peter Amstutz over 1 year ago

  • Related to Bug #20611: Creating api object hangs when inside crunch container added
Actions #3

Updated by Brett Smith over 1 year ago

20613-googleapiclient-init-logs @ d5749d0806f11da94e8ee3393b5166d4bba87198 - developer-run-tests: #3688

Brett Smith wrote:

  • Exact strategy TBD depending on whether logging objects are thread-safe, etc. It is acceptable if these logs only appear on the very first instantiation, but that limitation is not required either.

Logging to them is thread-safe but you need your own locking if you want to manipulate handlers, filters, etc. from threads. Given this, I went with a single-use lock strategy.

  • Remove the googleapiclient level-setting stuff from a-c-r.

Didn't remove it wholesale because it was doing some more subtle stuff I didn't appreciate but it has at least been updated to use the new tools.

Actions #4

Updated by Brett Smith over 1 year ago

Note 07ae0247ca233cd61d65dc600b8074c67291e3c0 with a brown paper bag fix.

Actions #5

Updated by Peter Amstutz over 1 year ago

  • Status changed from New to In Progress
Actions #6

Updated by Peter Amstutz over 1 year ago

  • Target version changed from Development 2023-06-07 to Development 2023-06-21 sprint
Actions #7

Updated by Brett Smith over 1 year ago

Actions #8

Updated by Peter Amstutz over 1 year ago

20613-googleapiclient-init-logs @ 3f23131d66a0467b2bee74006d61ec3e25a153bb

Since import arvados.logging is a new module and we get to name it whatever we want, would there be a benefit that instead of doing a dance in __init__.py we just name it something else to avoid deliberately conflicting with the standard library?

if logging.NOTSET < client_level < client_filter.retry_levelno:

Lots (most) programming languages don't do what this means in standard math notation, is Python one of the ones that does?

not client_logger.hasHandlers()

As discussed on matrix, hasHandlers turns out to always be True (so client_logger_unconfigured is never True) because googleapiclient sets up a NullHandler, which is apparently recommended practice.

Actions #9

Updated by Brett Smith over 1 year ago

Peter Amstutz wrote in #note-8:

Since import arvados.logging is a new module and we get to name it whatever we want, would there be a benefit that instead of doing a dance in __init__.py we just name it something else to avoid deliberately conflicting with the standard library?

IMO there is no name conflict between logging and arvados.logging. The reason the dance is necessary in __init__.py is because there's just way too much stuff in there. But doing it is worth it to make a nicer experience for users. Compare the situation with arvados.api, where it's literally impossible to import the module normally because __init__.py masks it with a function.

if logging.NOTSET < client_level < client_filter.retry_levelno:

Lots (most) programming languages don't do what this means in standard math notation, is Python one of the ones that does?

unlike C, expressions like a < b < c have the interpretation that is conventional in mathematics

not client_logger.hasHandlers()

As discussed on matrix, hasHandlers turns out to always be True (so client_logger_unconfigured is never True) because googleapiclient sets up a NullHandler, which is apparently recommended practice.

Thanks for catching this, I checked a-c-r but didn't think to check googleapiclient itself. Updated the check in 150de14b0b265d86df11a04201320944d04fe3a5 - developer-run-tests: #3691

Actions #10

Updated by Peter Amstutz over 1 year ago

Brett Smith wrote in #note-9:

Thanks for catching this, I checked a-c-r but didn't think to check googleapiclient itself. Updated the check in 150de14b0b265d86df11a04201320944d04fe3a5 - developer-run-tests: #3691

wb1 tests are failing but that's normal (for a deviant definition of normal)

Tested manually and now it does what it is supposed to do.

LGTM.

Actions #11

Updated by Brett Smith over 1 year ago

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

Also available in: Atom PDF