Conversation
68cb5c0 to
bcca125
Compare
Target repo exclusion is set only in clone operation and the pattern is stored in last_validated_commits file. Excluded repos' last validated commits are set to None. --exclude-target is removed in favor of --exclude-filter. --exclude-filter is extended to filter repos by name to match --exclude-target implementation.
0fbb74f to
ebc018c
Compare
| last_validated_data = client_auth_repo.last_validated_data | ||
| assert last_validated_data is not None | ||
| if last_validated_data.get("exclude_filter") is not None: | ||
| assert last_validated_data["exclude_filter"] |
There was a problem hiding this comment.
This assert does not really check anything given the if condition. Maybe if excluded: assert "exclude_filter" in last_validated_data else assert exclude_filter" not in last_validated_data would be more useful.
There was a problem hiding this comment.
mypy was complaining if there was no null check, so that's why I put it like that.
taf/updater/updater_pipeline.py
Outdated
| all_repositories = repositoriesdb.load_repositories_json( | ||
| self.state.users_auth_repo | ||
| ) | ||
| if all_repositories is not None: |
There was a problem hiding this comment.
This was my old way of handling this, so without saving exclude_filter. Just look for repositories not in last_validated_data and skip them. If are going to start storing exclude_filter, we should either validate rely on it primarily and just check if repositories not in last validated data and not on disk match the filter? What if there is a discrepancy here? Do we even need excluded_target_globs if we have exclude_filter? I added that automatic detection of excluded target repositories since there was no way to read this information.
There was a problem hiding this comment.
I just realized that we can get rid of this completely. I think that it's not likely to happen that repositories.json doesn't have one of the target repos as an entry?
There was a problem hiding this comment.
It very unlikely, but I wouldn't make that assumption.
0a44f03 to
4aef841
Compare
renatav
left a comment
There was a problem hiding this comment.
Tested and works well. Found a minor edge-case, might not even be addressing now. When you clone with an exclude-filter and then remove the filter line from the last_validated_commit file and run update, it clones the missing repository but prints All repositories are up-to-date with no changes or errors.
Description (e.g. "Related to ...", etc.)
Target repo exclusion is set only in clone operation and the pattern is stored in
last_validated_commitfile.Excluded target repos' last validated commits are set to
None.--exclude-targetis removed in favor of--exclude-filter.--exclude-filteris extended to filter repos by name to match--exclude-targetimplementation.In
auth_repo.py:Removed excluded repos handling for methods
targets_data_by_auth_commitandsorted_commits_and_branches_per_repositoriessince it is not used anywhere. Introducing--exclude-filterwould lead to coupling with repositoriesdb.Tests:
Removed
test_update_after_update_with_exclude,test_update_after_update_with_exclude_with_invalid_commitandtest_full_update_after_partial_updatesince update with exclude is no longer feasible.Added verification for expected
last_validated_commitentires -verify_excluded_lvc_entriesfunction.Closes: #716
Code review checklist (for code reviewer to complete)