-
Notifications
You must be signed in to change notification settings - Fork 297
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
Fixes #38117 - Refresh smart proxy sync history when proxy is updated #11270
Fixes #38117 - Refresh smart proxy sync history when proxy is updated #11270
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working well! Just one comment about calculating the sync histories to delete.
repos = smart_proxy_helper.repositories_available_to_capsule | ||
repo_ids = repos.map(&:id) | ||
if repo_ids.empty? | ||
self.smart_proxy_sync_histories.delete_all | ||
else | ||
self.smart_proxy_sync_histories.where("repository_id NOT IN (?)", repo_ids).delete_all | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repos = smart_proxy_helper.repositories_available_to_capsule | |
repo_ids = repos.map(&:id) | |
if repo_ids.empty? | |
self.smart_proxy_sync_histories.delete_all | |
else | |
self.smart_proxy_sync_histories.where("repository_id NOT IN (?)", repo_ids).delete_all | |
end | |
repos = smart_proxy_helper.repositories_available_to_capsule.select(:id) | |
# Use size to ensure repos don't get loaded unnecessarily | |
if repos.size == 0 | |
self.smart_proxy_sync_histories.delete_all | |
else | |
# Uses a single SQL query without needing to load IDs | |
self.smart_proxy_sync_histories.where(id: repos).delete_all | |
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack..Updated to this with slight change to maintain previous logic:
self.smart_proxy_sync_histories.where.not(repository_id: repos).delete_all
f764ef2
to
0792cf6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like tests broke, but the change on a running machine works well for me.
All relevant failures..Will update tests.. |
0792cf6
to
0292963
Compare
0292963
to
d55fe8e
Compare
What are the changes introduced in this pull request?
Refresh smart proxy sync history when sp is updated and only keep sync histories of repos available to capsule.
Considerations taken when implementing this change?
When LCE is removed from sp, we will now assume the sync history is not needed anymore.
What this does is that when env is added back, an optimized sync will not skip over this repo during sync.
What are the testing steps for this pull request?
The issue here is that optimized sync simply skips the repos from the environment cause their old sync histories have not been cleared.