Fix server error when using gpgcheck/repo_gpgcheck #3369
Conversation
|
If you do provide those options via the API, what happens? Do you get an error or are they simply ignored? |
|
@dralley You mean the REST Api? It's the same error as reported in the issue in both Rest and CLI (after getting that integer bugfix). I've also checked the test reproduces the bug. Edit: I've just tested that manually again and, although no error is raised, the option is not set in |
|
I meant "post-patch / post-PR, if you provide that option, does it yield an error or is it ignored?" Sounds like from your update it is ignored. I think technically, if they are ignored that would still be incorrect behavior. Deprecating a feature doesn't mean that it will immediately stop working, just that it could be removed at some later point. Fixing this completely would probably mean that instead of setting these fields The alternative would be removing the feature completely and immediately, but I'm not sure I'm comfortable with that. @ipanova Do you have an opinion? |
| @@ -0,0 +1,5 @@ | |||
| def test_create_repo_with_deprecated_gpg_options_3357(rpm_repository_factory): | |||
| """Can create repository with deprecated gpgcheck and repo_gpgcheck options.""" | |||
There was a problem hiding this comment.
We ought to test both reading and writing this value. I assume writing it will be broken - it won't error, but it also won't do what the user wanted.
There was a problem hiding this comment.
I've updated the tests and the approach to cover what you've pointed out. Do you think they are reflecting the intended workflow correctly?
If yes, I'll investigate the side-effects that are causing the CI to fail.
There was a problem hiding this comment.
This looks correct to me, yup. The failing test is interesting, it's not obvious why that would happen.
pedro-psb
force-pushed
the
fix/deprecated-gpg-opt-error
branch
from
04b238d to
9779aa9
Compare
pedro-psb
force-pushed
the
fix/deprecated-gpg-opt-error
branch
from
9779aa9 to
afc0fbc
Compare
Backport to 3.24: 💚 backport PR created✅ Backport PR branch: Backported as #3370 🤖 @patchback |
#3298 moved gpg options to a single field called "repo_config", but the deprecated "gpgcheck" and "repo_gpgcheck" were still being passed to the models, which caused a TypeError when trying to use them.
closes #3357