Update config module

[jonavellecuerdo]

Purpose and background context

Update the config module to align with some of our current conventions for instantiation, env var access, and logging controls. Though we have not standardized our approaches to managing app configurations, the following properties/features were implemented for DSS:

• Use a WARNING_ONLY_LOGGERS environment variable to minimize “noise” for select third-party libraries (e.g., [botocore, boto3, smart_open, urllib3]).
• DSS CLI supports a verbose flag.
• Config uses property methods to set defaults for optional environment variables and raise OSError for missing required environment variables.
• Methods configure_logger and configure_sentry are defined in the config module (but not as methods of the Config class itself).

How can a reviewer manually see the effects of these changes?

DSS was run in Dev using a sample OpenCourseWare item submission and a test collection in DSpace 8.

1. View CloudWatch log stream for a successful ingest: https://mit-dspace.eks.prod.4science.cloud/handle/1721.1/164334
   Note: Ignore the very first logged message with "VERBOSE: True"; was used for testing and has been removed.
2. View CloudWatch log stream for a failed item submission
   ✨ We can now see the logged exceptions from dspace-rest-python more easily
   Note: Recreated the error from IN-1687 by temporarily removing the DSS E-Person from the "Administrator" group in the DSpace 8 instance prior to running DSS.

Includes new or updated dependencies?

YES

Changes expectations for external applications?

YES; To see logs at DEBUG level, users and calling apps must set --verbose / -v CLI option.

What are the relevant tickets?

• https://mitlibraries.atlassian.net/browse/IN-1679

Code review

• Code review best practices are documented here and you are encouraged to have a constructive dialogue with your reviewers about their preferences and expectations.

[ehanson8]

Great work! A few questions and suggestions

[ehanson8]

Why change this from the Config attribute as the default? I thought our general practice was to only read env var in the Config class

[jonavellecuerdo]

I've seen envvar used more frequently in our more current applications, and this is the syntax for it: https://click.palletsprojects.com/en/stable/options/#values-from-environment-variables (i.e., it accepts a string).

The order of precedence in Click is:

• A value provided on the command line will override an environment variable or default value.
• An environment variable will override the default value defined in the option decorator.

[ehanson8]

It feels weird to me that we have all the logic in the Config module for processing env var and what to do if they're not there but then we're accessing directly through the CLI. Maybe this is a topic better reserved for a larger Config discussion but I would also appreciate @ghukill thoughts on this before proceeding

[jonavellecuerdo]

For completeness, we could set:

envvar="INPUT_QUEUE"
default=Config.input_queue

Where envvar is insufficient is if you have to do something with the environment variable, like splitting by a delimiter.

[ehanson8]

Same question as above

[jonavellecuerdo]

See response #59 (comment)!

[ehanson8]

What about a check_required_env_vars method like we have in DSC to eliminate some of these checks in the properties?

[jonavellecuerdo]

Hmm, I'd like to hold off on defining additional methods on the Config class until DataEng can have a more focused discussion on how we can standardize the format of this module (i.e., what methods to include, how to check required env vars).

In the case of development with DSC, I found it a hindrance that check_required_env_vars is called whenever you run the CLI, requiring the user to define all env vars even if only for local development.
In the latest version of python-lambdas-template, the method is defined, but it is not called when executed.

TLDR: Given that this works for now, I propose we move forward as is and discuss a better way to handle environment-specific required env var checking!

[ehanson8]

We can discuss a more standardized approach to the Config class soon so this is fine for now!