-
Notifications
You must be signed in to change notification settings - Fork 76
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
passing log candidates as batch for and copy and delete #7375
Conversation
83d50d5
to
787eb23
Compare
7bfa145
to
e569d0e
Compare
d4fb06e
to
f041762
Compare
1f06e01
to
a4b5719
Compare
7c08428
to
3d8f875
Compare
// if there is already a candidate for the same key, then we have a conflict | ||
if (logs_for_key_time_map.has(log.time)) { | ||
// If there is already a candidate for the same key, then we have a conflict | ||
if (logs_for_key_time_map.has(log_time)) { |
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.
Hey @Utkarsh-pro
Just for information while writing the unit tests I checked that this condition is not working as expected because of strict comparison, we are comparing two different time objects here which put this condition to always false
state. Updated the code with string comparison now working fine.
Pls let me know if you have any more thoughts. Thanks
11534bd
to
7921081
Compare
cf4191f
to
7b96749
Compare
ab96dd8
to
66c4423
Compare
for (const candidate of src_dst_objects_list) { | ||
await this.process_candidate(src_bucket, dst_bucket, candidate); | ||
const action = await this.process_candidate(candidate); |
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.
unless I missed anything, process_candidate
doesn't need to be async and there is no reason to wait for it.
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.
yes right, no need to async updated the code. Thanks
01e7594
to
4709a11
Compare
Signed-off-by: Aayush Chouhan <aayush.chouhan97@gmail.com>
Explain the changes