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

[Client Refactoring 6] Split download_certificates #365

Merged
merged 6 commits into from
Feb 5, 2022

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Feb 4, 2022

Split download_certificates into stateful and stateless parts, preparing for the final refactoring.

@lxfind lxfind force-pushed the download_owned_objects branch from 55d585a to 2eef81a Compare February 4, 2022 23:20
@lxfind lxfind force-pushed the split_download_certificates branch from 233d959 to b5563a3 Compare February 4, 2022 23:22
Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +853 to +865
(
(object_id, next_sequence_number),
self.certificates(&object_id)
.flat_map(|cert| cert.order.input_objects())
.filter_map(|object_kind| {
if object_kind.object_id() == object_id {
Some(object_kind.version())
} else {
None
}
})
.collect::<HashSet<_>>(),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this seems marginally more readable

let versions = self.certificates(&object_id) ..;
((object_id, next_sequence_number), versions)

Base automatically changed from download_owned_objects to main February 5, 2022 16:33
* [Client Refactoring 7] Lock/unlock order upfront

* [Client Refactoring 8] Add AuthorityAggregator (#364)

* [Client Refactoring] Add AuthorityAggregator

* [Client Refactoring 9] Make download_objects_not_in_db fast again (#368)

* [Client Refactoring 9] Make download_objects_not_in_db fast again

* [Clienet Refactoring 10] Properly lock orde forr transfer_to_fastx_unsafe_unconfirmed (#369)
* [Client Refactoring 7] Lock/unlock order upfront

* [Client Refactoring 8] Add AuthorityAggregator (#364)

* [Client Refactoring] Add AuthorityAggregator

* [Client Refactoring 9] Make download_objects_not_in_db fast again (#368)

* [Client Refactoring 9] Make download_objects_not_in_db fast again

* [Clienet Refactoring 10] Properly lock orde forr transfer_to_fastx_unsafe_unconfirmed (#369)
@lxfind lxfind force-pushed the split_download_certificates branch from db05ab4 to b1e2410 Compare February 5, 2022 17:35
@lxfind lxfind merged commit 8df9755 into main Feb 5, 2022
@lxfind lxfind deleted the split_download_certificates branch February 5, 2022 17:51
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.

3 participants