Hotfix: Exponential Backoff TypeError

[jayckaiser]

Hotfix: Exponential Backoff (NoneType > Int) TypeError

Description & motivation

This PR resolves edu_edfi_airflow Issue #138.

There are two ways to initialize an EdFiSession:

• EdFiClient.connect(): Intended approach (passes connection arguments into a new Session)
• EdFiClient init: Builds a lazy session object that ignores retry arguments, expecting them to be overridden during get_* calls.

Our current version of EdfiHook in edu_edfi_airflow uses the second method (i.e., it initializes a lazy session object but does not populate retry arguments). This led to a state where variables like max_wait were never initialized, leading to NoneType errors during exponential backoff during ingestion.

This PR simply updates the init for EdFiSession to populate global retry arguments when they're passed directly as kwargs during init.

PR Merge Priority:

This is a breaking bug that prevents exponential backoff from running during production ingestions.

• Low
• Medium
• High

• edfi_api_client/session.py: Populate instance attributes with kwargs when passed, instead of always defaulting to None.

Tests and QC done:

Run successfully in SC prod. (Note that more runs are needed to ensure this has fully worked as intended.)

[jayckaiser]

Update: I am still seeing the same error in long-running endpoint-ingestions. This fix is incomplete.

I will outline below the lineage of how these methods are called from the EdFiResourceDAG to help others investigate this issue:

edu_edfi_airflow.EdFiResourceDAG
    edu_edfi_airflow.EdFiToObjectStorageOperator.execute()

        edu_edfi_airflow.EdFiHook
            edfi_api_client.EdFiClient
                edfi_api_client.EdFiSession.connect()  -> Lazy init with no kwargs

        edu_edfi_airflow.EdFiToObjectStorageOperator.pull_edfi_to_object_storage()
            edfi_api_client.EdFiClient.resource()
                edfi_api_client.EdFiResource.get_rows()
                    edfi_api_client.EdFiSession.get_response()

Lots to step through here, but TL;DR: we never explicitly define or pass max_wait or num_retries through to EdFiClient. EdFiSession initializes with an empty self.max_wait that should be updated on EdFiSession.connect(), but this does not appear to be happening. Therefore, in EdFiSession._with_exponential_backoff(), the line max_wait = max_wait or self.max_wait boils down to max_wait = None or None.

I'm confusing myself as I walk through the lineage here, but we need to force these arguments to always be defined at least once during init.

[alchenist]

We're hitting this because the code that accesses self.max_wait in the exponential backoff wrapper runs before an EdFiSession that's initialized with a max_wait of None ever gets to EdFiSession.connect() -- that call is further down in the wrapped function. What's the use case for initializing a session with undefined retry arguments (max_retries, max_wait)?

One option that immediately comes to mind is to assign defaults in EdFiSession.init directly. These could still get overridden permanently if a caller uses EdFiSession.connect AND overridden on a call-by-call basis during get_* calls. Typing would also be a little easier if we can avoid effectively typing both max_retries and max_wait as Optional[int]s. I feel like I'm missing a good historical reason to keep these lazy.