[RHELC-1507] Port environment variables to config file #1272
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
edd1500
to
66e7744
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1272 +/- ##
==========================================
+ Coverage 96.34% 96.52% +0.17%
==========================================
Files 66 70 +4
Lines 4983 5065 +82
Branches 877 881 +4
==========================================
+ Hits 4801 4889 +88
+ Misses 102 98 -4
+ Partials 80 78 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
66e7744
to
a7904cf
Compare
8 tasks
hosekadam
reviewed
Jul 22, 2024
hosekadam
reviewed
Jul 22, 2024
hosekadam
reviewed
Jul 22, 2024
hosekadam
reviewed
Jul 23, 2024
hosekadam
reviewed
Jul 23, 2024
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
8ffaf85
to
f596240
Compare
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
f596240
to
1330775
Compare
r0x0d
commented
Aug 6, 2024
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
22f587c
to
9639951
Compare
r0x0d
added
kind/feature
|
/packit build |
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
9639951
to
edb3a8d
Compare
|
/packit test --labels tier0 |
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
edb3a8d
to
c742c76
Compare
|
/packit test --labels tier0 |
Venefilyn
reviewed
Aug 8, 2024
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
2 times, most recently
from
312f34d
to
e217b35
Compare
|
/packit build |
|
/packit test --labels tier0 |
|
/packit test --labels tier0 |
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
5ad7c8c
to
2478c25
Compare
|
/packit test --labels tier0 |
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
2478c25
to
24e1b7f
Compare
|
/packit test --labels tier0 |
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
24e1b7f
to
2560fba
Compare
|
/packit test --labels tier0 |
1 similar comment
|
/packit test --labels tier0 |
kokesak
reviewed
Sep 18, 2024
There was a problem hiding this comment.
Tests are passing. But I think we can extend our test suite. In integration tests that are verifying a certain inhibitor and ability to override it, we could also introduce a variant that puts the override value to the config file instead of the environment. I wouldn't block this PR on this - we can add this in the integration test ticket - https://issues.redhat.com/browse/RHELC-1535
In the past, we used to have this portion of code all together inside the toolopts module, making it very difficult to understand and improve as the separation was not intuitive enough. With this patch, we are splitting the cli related functions and classes to its own module, making it an entrypoint for everything related to CLI usage. Toolopts also got its own module to further separate the logic between cli stuff and options stuff. Historically we call all options from our source code as "toolopts", so to preserve the naming convention and minimize as much as possible the changes throughout the codebase, the same name was preserved. A new addition was also made which is the config module. This module is responsible to gather all the configuration options we could ever define for the tool. Currently, it only has CLIConfig and FileConfig, which are the two supported options. Toolopts, at this point, is our final class who will hold all the options that are composed from the cli and config file (being able to be extended in the future to other options).
A couple of functions and constants were moved to their related utils modules to get them out of the way from circle dependency and make the codebase a bit more clear
This patch introduces the updated references to both toolopts and cli throughout the codebase as some of the things we had before could be changed to the new format, especially because no code get executed before the CLI class parse all the options necessary. We are not safe to call tool_opts even if nothing is populated inside the class. Beware that it will still fail if we try to access any attributes before they get created, but that should be a implementation problem, not really a workflow one, since we expect that the CLI get executed very early in the process.
Add a couple of missing variables to the vulture whitelist script so the hook can stop warning about unused variable.
Parsing the values for inhibitor_overrides now convert it to a boolean value by default.
r0x0d
force-pushed
the
port-env-vars-to-config-file
branch
from
2d4a4cf
to
b4e7e61
Compare
|
/packit test --labels tier0 |
kokesak
approved these changes
Sep 19, 2024
Venefilyn
approved these changes
Sep 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch introduces the initial work to port the environment variables
to be read by a configuration file, parsed, and put into the ToolOpts
class.
With that, we are aiming to remove the environments variables completely
in the next release, managing all the configurations/inhibitors/settings
through that config file.
Jira Issues:
Checklist
[RHELC-]or[HMS-]is part of the PR titleRelease Pendingif relevant