-
Notifications
You must be signed in to change notification settings - Fork 10
Shutdown and termination sig handling #15
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
Shutdown and termination sig handling #15
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 |
8f8421b to
999c8dd
Compare
|
Hi @ShoroukRamzy, I think the problem is that the crate |
Hi @armin-acn, I have opened this PR to add ctrlc to score crates eclipse-score/score-crates#15? |
|
Approved it. |
Thanks @armin-acn, I'll retest after the PR merge. |
|
Hi @ShoroukRamzy, would it be a problem for you, if the very first Release of FEO v0.5 (to be prepared until tomorrow the latest) would not include this PR? We can probably create a minor release update very soon, which could then include your changes. |
Hi @armin-acn, Yes, sure. We can do this. |
|
Hi @armin-acn, |
|
Hi Shorouk, you should not need to change the existing code, as it did pass the linter. But from your last commit it looks like you are already progressing. |
264e241 to
c2beeca
Compare
|
Hi @armin-acn, |
| copyright_checker( | ||
| name = "copyright", | ||
| srcs = [ | ||
| "src", |
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.
Why is this needed?
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 src dir is empty after feo migration, I removed it. do I need to keep 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.
Which src dir do you mean? I guess it checks every src dir in the repository, but I don't know for sure how 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.
https://github.com/eclipse-score/feo/tree/main/src this one I mean
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.
Ok, I don't think it's related to this directory only. Let's not change it in this commit as the change is unrelated.
examples/rust/mini-adas/Cargo.toml
Outdated
| default = ["com_iox2", "signalling_relayed_tcp"] | ||
| signalling_direct_mpsc = [] | ||
| signalling_direct_tcp = [] | ||
| signalling_direct_tcp = ["com_iox2"] |
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.
Why?
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.
Thanks , you are right I added it during direct signalling testing but there is no need for it, I will remove 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.
Done
feo/src/scheduler.rs
Outdated
| } | ||
| break id; | ||
| } | ||
| Some(Signal::TerminateAck(_)) => continue, // Ignore during normal operation |
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.
Why ignoring unexpected signal without logging?
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.
Thanks, I will add logging info
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
c2beeca to
c22bffd
Compare
c22bffd to
0363e7c
Compare
artemsheinacn
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.
LGTM
Hi @armin-acn,
This is a continuation for this PR #12.
As per our discussion in the meeting this commit is not working after rebasing due to ctrlc issue.
Thank you for your support.