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

Enhance CI Workflow for Scenic Simulators: Improved Volume Management, Reliability, and Cost Efficiency #310

Merged
merged 38 commits into from
Nov 1, 2024

Conversation

lola831
Copy link
Collaborator

@lola831 lola831 commented Oct 29, 2024

Description

This PR updates the CI workflow for Scenic simulators to:

  • Create and attach a new gp3 volume from the latest snapshot during EC2 instance setup.

  • Prevent broken pipe errors during CARLA connection.

  • Ensure EC2 instance stops even if tests fail, and perform volume cleanup to avoid AWS costs.

  • Increase CARLA connection timeouts to ensure tests pass reliably.

Issue Link

This will resolve issue #293.

Checklist

  • I have tested the changes locally via pytest and/or other means
  • I have added or updated relevant documentation
  • I have autoformatted the code with black and isort
  • I have added test cases (if applicable)

Additional Notes

SCENIC-50

…rts to stop the instance. Added if: always() so that stop_ec2_instance always runs even when test fail
…attatch to instance before starting instance
- Added snapshot readiness check for volume creation
- Enhanced logging for detailed instance state tracking
- Extended timeout for instance status checks to 10 minutes
- Extended CARLA server connection attempt loop from 30 to 120 seconds.
- Added pytest failure if unable to connect to CARLA within 2 minutes.
- Increased CarlaSimulator communication timeout from 10 to 60 seconds.
@lola831 lola831 requested a review from dfremont October 29, 2024 21:53
@lola831 lola831 closed this Oct 29, 2024
@lola831 lola831 reopened this Oct 29, 2024
Copy link
Collaborator

@dfremont dfremont left a comment

Choose a reason for hiding this comment

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

Looks great, thanks Lola! Just a couple small suggestions above.

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.31%. Comparing base (0abbbcd) to head (3250886).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #310      +/-   ##
==========================================
- Coverage   91.37%   91.31%   -0.07%     
==========================================
  Files          54       54              
  Lines       13544    13537       -7     
==========================================
- Hits        12376    12361      -15     
- Misses       1168     1176       +8     

see 6 files with indirect coverage changes

@dfremont dfremont merged commit b0093d9 into main Nov 1, 2024
26 checks passed
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.

2 participants