Skip to content

Conversation

scanon
Copy link
Contributor

@scanon scanon commented Jul 10, 2023

This refactors the configuration handling to use a class instead of a dict. So this is a large change because this weaves through out the platform.

JobRunner/config.py add a Config class. This is initialized by the CLI tools or modules. The config object is then passed off to the other modules. Many of the tests also needed to be updated since they were initializing a configuration dict before.

There are a few minor flake8 fixes too but this is primarily what is needed to make the configuration change and fix tests.

bio-boris and others added 13 commits May 8, 2020 17:53
DEVOPS-515 decrease_frequency_of_monitor
Support differing ee2 urls
This refactors the configuration handling to use a class instead
of a dict.  So this is a large change because this weaves through
out the platform.  There are a few minor flake8 fixes too but
this is primarily what is needed to make the configuration change
and fix tests.
Also fixed an error in the JobRunner init
Copy link
Member

@MrCreosote MrCreosote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review was more about code structure / etc vs. correctness, since I'm not at all an expert on this code base.

I went through the entire set of changes, but there's probably some stuff that I missed. This is a lot of comments already so I'll stop here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here re repetitive code

scanon and others added 6 commits July 29, 2023 10:56
Co-authored-by: MrCreosote <MrCreosote@users.noreply.github.com>
Co-authored-by: MrCreosote <MrCreosote@users.noreply.github.com>
Co-authored-by: MrCreosote <MrCreosote@users.noreply.github.com>
@scanon scanon merged commit 6f18521 into callback_module Jul 29, 2023
@scanon scanon deleted the scanon/refactor_config branch August 1, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants