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

Distinct trust check #755

Merged
merged 5 commits into from
Feb 1, 2023
Merged

Distinct trust check #755

merged 5 commits into from
Feb 1, 2023

Conversation

jw3
Copy link
Member

@jw3 jw3 commented Feb 1, 2023

Improve #691 by adding ability to distinguish between type of trust being checked.

Rather than add a flag to the type, or separate return batches, this adds separate entrypoints. The callback handlers will be responsible for knowing which type they are receiving.

This also changes the return value of the check_* call to represent the total number of records to be checked. The count parameter of the update callback is now the total number checked as of that callback, such that the check_* return value should equal the callback count value on the final batch.

See the updated reference impl in examples/async_trust.py

#690
#750

@jw3 jw3 added the trust label Feb 1, 2023
@jw3 jw3 requested a review from dorschs57 February 1, 2023 14:09
@jw3
Copy link
Member Author

jw3 commented Feb 1, 2023

@dorschs57 what do you think about this approach? I was thinking that you could just fire up a check on ancillary and system at the same time to feed separate progress indicators on each view.

Comment on lines 62 to 65
eprintln!(
"BatchConf: tc:{}, tl{}, bc:{}, bl:{}",
batch_cfg.thread_cnt, batch_cfg.thread_load, batch_cfg.batch_cnt, batch_cfg.batch_load
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to leave this in here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, the stderr prints are placeholders for real log calls

Copy link
Collaborator

@dorschs57 dorschs57 left a comment

Choose a reason for hiding this comment

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

I like this approach. It will allow me to keep the 2 separated at the redux layer like they where in the original code. 👍

@jw3
Copy link
Member Author

jw3 commented Feb 1, 2023

I like this approach. It will allow me to keep the 2 separated at the redux layer like they where in the original code. +1

OK. I need to get one more test in to make sure the gil is synchronizing the merge call. Then it will be merged.

@jw3 jw3 merged commit 8b9749a into ctc-oss:master Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants