Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't fail when removing a content credential from a repository #1649

Merged
merged 2 commits into from
Aug 15, 2023

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Jul 25, 2023

No description provided.

@evgeni evgeni changed the title don't fail when searching for a content credential for a repository don't fail when removing a content credential from a repository Jul 25, 2023
@evgeni
Copy link
Member Author

evgeni commented Jul 25, 2023

probably should add tests, right? 🙈

@evgeni
Copy link
Member Author

evgeni commented Jul 26, 2023

and the tests say I am stupid…

and the tests helped me to write the fix correctly

@evgeni evgeni marked this pull request as draft July 26, 2023 12:45
@evgeni evgeni marked this pull request as ready for review July 26, 2023 13:09
@evgeni
Copy link
Member Author

evgeni commented Jul 27, 2023

Mhh. The existing apidoc doesn't have https://projects.theforeman.org/issues/36497 yet. Makes sense.

@evgeni
Copy link
Member Author

evgeni commented Aug 14, 2023

@ehelms @mdellweg @Griffin-Sullivan does anyone want to look at this one? 🥺

Copy link
Contributor

@Griffin-Sullivan Griffin-Sullivan left a comment

Choose a reason for hiding this comment

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

LGTM. Is the /fixtures directory what we use for "recorded" testing?

@evgeni
Copy link
Member Author

evgeni commented Aug 14, 2023

LGTM. Is the /fixtures directory what we use for "recorded" testing?

Yes, exactly.

@_check_patch_needed(fixed_version='3.8.0', plugins=['katello'])
def _patch_products_repositories_allow_nil_credential(self):
"""
This is a workaround for the missing allow_nil: true in the Products and Repositories controllers
Copy link
Member

Choose a reason for hiding this comment

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

This is a in two-three releases of Katello this work around gets dropped?

Copy link
Member Author

@evgeni evgeni Aug 14, 2023

Choose a reason for hiding this comment

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

We've been rather conservative in dropping those apidoc patches, but the code is a noop once it detects a new enough version.

def _check_patch_needed(introduced_version=None, fixed_version=None, plugins=None):
"""
Decorator to check whether a specific apidoc patch is required.
:param introduced_version: The version of Foreman the API bug was introduced.
:type introduced_version: str, optional
:param fixed_version: The version of Foreman the API bug was fixed.
:type fixed_version: str, optional
:param plugins: Which plugins are required for this patch.
:type plugins: list, optional
"""
def decor(f):
@wraps(f)
def inner(self, *args, **kwargs):
if plugins is not None and not all(self.has_plugin(plugin) for plugin in plugins):
return
if fixed_version is not None and self.foreman_version >= LooseVersion(fixed_version):
return
if introduced_version is not None and self.foreman_version < LooseVersion(introduced_version):
return
return f(self, *args, **kwargs)
return inner
return decor

@evgeni evgeni merged commit 2581555 into theforeman:develop Aug 15, 2023
21 checks passed
@evgeni evgeni deleted the bz2224122 branch August 15, 2023 06:06
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