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

CLOUDDST-24254 : Merge index image failing with FBC operator present … #808

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashwini3326
Copy link

This commit fix this bug by ignoring deprecation operators which are not available in database.
@release-engineering/exd-guild-hello-operator

@ashwini3326 ashwini3326 force-pushed the fix_merge_index_issue branch 2 times, most recently from c0ee208 to b803aa2 Compare February 5, 2025 03:11
iib/workers/tasks/utils.py Outdated Show resolved Hide resolved
@ashwini3326 ashwini3326 force-pushed the fix_merge_index_issue branch 4 times, most recently from 083f509 to 1b1b5de Compare February 6, 2025 11:25
@@ -1265,3 +1265,31 @@ def get_bundle_metadata(
for pullspec in operator_csv.get_pullspecs():
bundle_metadata['found_pullspecs'].add(pullspec)
return bundle_metadata


def filter_deprecation_operator_from_db(
Copy link
Contributor

Choose a reason for hiding this comment

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

I am thinking about this name - if I did not know more I would think that the function will filter out the operators from DB (that it will remove them from DB) but we want to get only list of operators present in DB.

what about something like get_operators_present_in_db or filter_operators_present_in_db or just filter_operators ?
@chandwanitulsi is good with naming :) maybe she could suggest something.

Copy link
Author

Choose a reason for hiding this comment

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

going with filter_operators_present_in_db

:param str temp_dir: temp directory where opm will be executed

:return: A List of operator bundle to be deprecated
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

missing :rtype: list(str)

Copy link
Author

Choose a reason for hiding this comment

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

thanks for the catch.

filter(lambda bundlepath: bundlepath in available_bundlepaths, deprecation_bundles)
)

log.info(f"Deprecation bundle after filter is : {deprecation_bundles}")
Copy link
Contributor

Choose a reason for hiding this comment

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

For logging we should not use f-string formatting but %s notation because logging lib is optimized for it.
https://docs.python.org/3/howto/logging-cookbook.html#formatting-styles

Copy link
Author

Choose a reason for hiding this comment

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

fixed, will reflect in next push.

bundles_list = get_list_bundles(index_db_file, temp_dir)

available_bundlepaths = []
for bundle in bundles_list:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we do the filtering in one cycle? Something like:

available_bundlepaths = [
bundle["bundlePath"]  for bundle in bundles_list if bundle["bundlePath"] in deprecation_bundles
]

then just log and return available_bundlepaths

Copy link
Author

Choose a reason for hiding this comment

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

changed, will reflect in next push.



@mock.patch('iib.workers.tasks.utils.get_list_bundles')
def test_no_matching_filter_deprecation_operator_from_db_(mock_bundle_list):
Copy link
Contributor

Choose a reason for hiding this comment

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

extra _ at the end of name test_no_matching_filter_deprecation_operator_from_db_

Copy link
Author

Choose a reason for hiding this comment

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

removed, will reflect in next push.

]

deprecation_bundles = [
"op1@sha",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think you could add "op3@sha", here as well?
The result should be the same

Copy link
Author

Choose a reason for hiding this comment

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

It makes unittest more specific, so I thought of not testing both cases in single unitest.

]

deprecation_bundles = [
"op3@sha",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this tests if we include this in the previous one?

…in source

This commit fix this bug by ignoring deprecation operators which are not available in database.
@ashwini3326 ashwini3326 force-pushed the fix_merge_index_issue branch from 1b1b5de to a2c2e17 Compare February 7, 2025 17:15
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.

2 participants