-
Notifications
You must be signed in to change notification settings - Fork 2
worker: Expose safety worker queue size config #50
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
worker: Expose safety worker queue size config #50
Conversation
License Check Results🚀 The license check job ran with the Bazel command: bazel run //:license-checkStatus: Click to expand output |
|
The created documentation from the pull request is available at: docu-html |
64e8d42 to
311e81b
Compare
311e81b to
1f1c406
Compare
| } | ||
| quote! { #e } | ||
| } | ||
| None => quote! { 64 }, // default |
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.
this 64 is sued here and in other places. We shall use single source const value in all cases for default to keep it consistent.
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.
as before SAFETY_QUEUE_SIZE
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.
Not fixed to avoid circular dependency because kyron has dependency on kyron-macros.
Note:
If SAFETY_QUEUE_SIZE is defined in kyron and if this dependency is added in kyron-macros, it results in circular dependency.
| /// Configure safety worker task queue size with `size`. | ||
| /// >ATTENTION: `size` has to be power of two | ||
| /// | ||
| pub fn safety_worker_task_queue_size(mut self, size: u32) -> Self { |
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.
this shall fail if enable_safety_worker was not called before or ? Otherwise user may thing it works.
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.
done
pawelrutkaq
left a comment
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.
and i did not spot adaptations for failing tests as in ticket? why is that ?
1f1c406 to
4e66384
Compare
4e66384 to
c746b27
Compare
Sorry missed it. Updated now. |
Expose safety worker task queue size configuration to user.
c746b27 to
bc645b8
Compare
|
@pawelrutkaq tests ok |
Expose safety worker task queue size configuration to user.
Notes for Reviewer
Pre-Review Checklist for the PR Author
Checklist for the PR Reviewer
Post-review Checklist for the PR Author
References
Closes #19