Deprecate Tags and Flags #788
Conversation
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
To launch regression testing public members of oamg organization can leave the following comment:
Please open ticket in case you experience technical problem with the CI. (RH internal only) Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra. |
|
Can one of the admins verify this patch? |
5a36d65 to
37867ab
Compare
|
/rerun |
|
Copr build succeeded: https://copr.fedorainfracloud.org/coprs/build/4687736 |
|
Testing Farm request for RHEL-8.6-rhui/4685924;4687736 regression testing has been created. |
|
Testing Farm request for RHEL-7.9-rhui/4685924;4687736 regression testing has been created. |
|
Testing Farm request for RHEL-8.6.0-Nightly/4685924;4687736 regression testing has been created. |
|
Testing Farm request for RHEL-7.9-ZStream/4685924;4687736 regression testing has been created. |
|
@fernflower also, if I understand the impact right, this change requires update of all actors now as well. Otherwise users will see a lot of deprecation reports each run of leapp |
Should be ok now https://github.com/oamg/leapp/blob/e2505c40e5a91ae6729e3564553fc3046242381d/docs/source/deprecation.md |
| @@ -9,6 +9,7 @@ | |||
| from leapp.models import fields, Model, ErrorModel | |||
| from leapp.topics import ReportTopic | |||
| from leapp.libraries.stdlib.api import produce | |||
| from leapp.utils.deprecation import deprecated | |||
There was a problem hiding this comment.
if we are not going to change actors now, the code change must be skipped. otherwise actors need to be updated first. tbh, i prefer to update the actors. it's not so be deal a believe.
There was a problem hiding this comment.
I have been down this road (all actor code update with Tags/Flags->Groups transition) with previous POC and unfortunately it is a big deal and it's getting worse with each new actor pr created or merged :(
If we are to do this - I will definitely need help. It's not only about actors, it's also about tests, and it's not only dumb substitution, but meaningful refactoring as well.
There was a problem hiding this comment.
I don't want to rush code update to be honest, I'd rather hack our deprecation procedure a little to allow room for not-raising-a-warning-for-this-specific-model.
There was a problem hiding this comment.
@fernflower that would mean to update each our actor anyway to suppress the warning - which means we can already update actors directly. creating additional mechanism on top of what exists already does not make sense from my pov and more logically could be to drop the code change raising the warning. The code change seems pretty simple in actors so I would go with that. I am going to open the pr
There was a problem hiding this comment.
@fernflower I have opened the draft PR with the applied changes, so we could check how much stuff it affects in existing tests to have some data what else will be possibly needed on top of that change. See oamg/leapp-repository#932
In case we find there a lot of problems around, I would drop the change in the code regarding the deprecation directives, so customer will not see bunch of deprecations warnings. In case we find cheap to do the change, I prefer to do it. wdyt about that?
There was a problem hiding this comment.
P.S. I know that in this case we will have to deal with mass-lint-fix as well. But when we are already in state that change of majority actors is inevitable, I think it's good time to deal with that as well.
1694605 to
d134f2f
Compare
Apply official deprecation process for Tags and Flags primitives that has been superseeded by Groups. This change includes the update of the documentation that is source of truth regarding the deprecated functionality. OAMG-7262
d134f2f to
d37c513
Compare
|
@fernflower I rebased & squashed commits. I was missing blob to be able to test qith up-to-date master |
|
With the old leapp-repository build, bunch of deprecation reports are generated, which means the deprecation delivered in this commit works as expected: Jut for the completness, with the current upstream leapp-repository builds, no deprecation warning have been raised, so current changes in leapp-repository are correct as well. |
|
Merging!! @fernflower congratulation!! Happy to have this one long story around Flags/Tags finally finished after almost 2y 🎆 🍾 |
## Packaging - bumped leapp-framework to 3.1 (oamg#677) ## Framework ### Fixes - Fixed a problem where passing environment variables to an executed child process modified the environment variables of the parent process (oamg#784) - Ignore invalid FQDNs (oamg#790) ### Enhancements - Deprecate `reporting.(Tags|Flags)`, replaced by `reporting.Groups` (oamg#677, oamg#781, oamg#788) - Introduce `is_inhibitor` function (oamg#677) - Introduce a `Blob` model field (oamg#789) - Introduce new report JSON schema v1.2.0 (default: 1.1.0) (oamg#677) ## Leapp (tool) ### Fixes - Handle missing CLI commands gracefully (oamg#785) - Requires to be executed by root only (oamg#775)
## Packaging - bumped leapp-framework to 3.1 (oamg#677) ## Framework ### Fixes - Fixed a problem where passing environment variables to an executed child process modified the environment variables of the parent process (oamg#784) - Ignore invalid FQDNs (oamg#790) ### Enhancements - Deprecate `reporting.(Tags|Flags)`, replaced by `reporting.Groups` (oamg#677, oamg#781, oamg#788) - Introduce `is_inhibitor` function (oamg#677) - Introduce a `Blob` model field (oamg#789) - Introduce new report JSON schema v1.2.0 (default: 1.1.0) (oamg#677) ## Leapp (tool) ### Fixes - Handle missing CLI commands gracefully (oamg#785) - Requires to be executed by root only (oamg#775) Signed-off-by: Petr Stodulka <pstodulk@redhat.com>
## Packaging - bumped leapp-framework to 3.1 (#677) ## Framework ### Fixes - Fixed a problem where passing environment variables to an executed child process modified the environment variables of the parent process (#784) - Ignore invalid FQDNs (#790) ### Enhancements - Deprecate `reporting.(Tags|Flags)`, replaced by `reporting.Groups` (#677, #781, #788) - Introduce `is_inhibitor` function (#677) - Introduce a `Blob` model field (#789) - Introduce new report JSON schema v1.2.0 (default: 1.1.0) (#677) ## Leapp (tool) ### Fixes - Handle missing CLI commands gracefully (#785) - Requires to be executed by root only (#775) Signed-off-by: Petr Stodulka <pstodulk@redhat.com> Signed-off-by: Petr Stodulka <pstodulk@redhat.com>
Apply official deprecation process for Tags and Flags
primitives that has been superseeded by Groups.
OAMG-7262