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

ReadyToTest action timeout using decorator #625

Merged
merged 10 commits into from
Jul 11, 2022

Conversation

deepanshubansal01
Copy link
Contributor

@deepanshubansal01 deepanshubansal01 commented Jul 8, 2022

This PR is related to #582

By default the ReadyToTest action waits for 15 sec for the processes to start up, which means the processes that takes longer to start up, won't be able to start up. This PR adds a configurable timeout duration for ReadyToTest action by supporting a new decorator namely ready_to_test_action_timeout

Some discussion here #622

Signed-off-by: deepanshu [email protected]

Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM!

I have some minor comments

@ivanpauno ivanpauno requested a review from hidmic July 8, 2022 18:37
@ivanpauno ivanpauno added the enhancement New feature or request label Jul 8, 2022
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

LGTM with green CI!

@deepanshubansal01
Copy link
Contributor Author

deepanshubansal01 commented Jul 8, 2022

LGTM with green CI!

LGTM with green CI!

Thanks a lot for the review, can I merge this now or wait for some more reviewers?

@ivanpauno
Copy link
Member

You can merge with one approval.
Check the Rpr failure (this one) and then run CI.
If everything is green you can merge.

Signed-off-by: Aditya <[email protected]>
@adityapande-1995
Copy link
Contributor

adityapande-1995 commented Jul 8, 2022

CI :
build : --packages-above-and-dependencies launch_testing
test : --packages-above launch_testing

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Signed-off-by: Aditya <[email protected]>
@adityapande-1995
Copy link
Contributor

adityapande-1995 commented Jul 9, 2022

CI round 2 :

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@tonynajjar
Copy link
Contributor

I need this on humble, any reason not to backport? I could cherrypick it with no conflicts

tonynajjar pushed a commit to angsa-robotics/launch that referenced this pull request Apr 2, 2024
* Added support for configurable timeout duration for ReadyToTest action using decorator 

Signed-off-by: deepanshu <[email protected]>

Co-authored-by: Aditya <[email protected]>
@christophfroehlich
Copy link

I need this on humble, any reason not to backport? I could cherrypick it with no conflicts

ping @deepanshubansal01, we'd also use this on humble. any chance for backporting it? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants