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.

Sign up for GitHub

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

Jump to bottom

[openshift-tekton-resources] early-exit #4656

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[openshift-tekton-resources] early-exit #4656

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68759ae
openshift-tekton-resources early-exit
maorfr Sep 10, 2024
b677614
fix
maorfr Sep 10, 2024
6fad5ed
sort
maorfr Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions reconcile/openshift_tekton_resources.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,3 +464,7 @@ def run(
sys.exit(ExitCodes.ERROR)

sys.exit(0)


def early_exit_desired_state(*args: Any, **kwargs: Any) -> dict[str, Any]:
return fetch_tkn_providers(saas_file_name=None)
Copy link
Contributor

Choose a reason for hiding this comment

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

the order of saas files in dict values may affect equal check in different run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you explain more?

Copy link
Contributor

Choose a reason for hiding this comment

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

fetch_tkn_providers(saas_file_name=None) returns a dict, key is provider name, value is a list of saas files

saas_files = fetch_saas_files(saas_file_name)

the order of saas files returned from gql maybe different, lead to desired state not equal to last one even if no saas file changed, may need to change it to dict[str, set[str]] to be stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have we seen this behavior in the past?

Copy link
Contributor

@hemslo hemslo Sep 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

yes, changed many early exit state from list to set/dict for diff issues

Copy link
Contributor

Choose a reason for hiding this comment

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

dict keys order doesn't matter, just list values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wdyt 6fad5ed?

Copy link
Contributor

Choose a reason for hiding this comment

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

since here we are not using typed query, saas_files is added to gql response dynamicly, it should be safe to use set here, unless we want to keep it ordered for other purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

either

    for provider_name in tkn_providers:
        tkn_providers[provider_name]["saas_files"].sort(key=lambda sf: sf["name"])

or

        if "saas_files" not in tkn_providers[provider_name]:
            tkn_providers[provider_name]["saas_files"] = set()

        tkn_providers[provider_name]["saas_files"].add(sf)

no need to sorted for dict keys.