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.

Sign up for GitHub

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

Jump to bottom

Add log file cleanup to dragon entrypoint #554

Merged
merged 4 commits into from
Apr 23, 2024
Merged

Add log file cleanup to dragon entrypoint #554

merged 4 commits into from
Apr 23, 2024

Conversation

ankona
Copy link
Contributor

@ankona ankona commented Apr 17, 2024 •
edited
Loading

Update the dragon entrypoint to ensure that the log file is removed when the environment is shutdown.

Additional updates:

  • minor refactor to enable testing entrypoint features
  • add tests for entrypoint functions
  • update incorrect license clause

@ankona ankona changed the title checkpoint Add log file cleanup to dragon entrypoint Apr 17, 2024
@ankona ankona requested a review from al-rigazzi April 17, 2024 22:20
al-rigazzi
al-rigazzi approved these changes Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

LGTM, this will save us a lot of time in tests!

tests/on_wlm/test_dragon.py
@@ -1,6 +1,6 @@
# BSD 2-Clause License
#
# Copyright (c) 2021-2023, Hewlett Packard Enterprise
# Copyright (c) 2021-2024, Hewlett Packard Enterprise
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm so old school.

@ankona ankona added type: feature Issues that include feature request or feature idea type: refactor Issues focused on refactoring existing code area: launcher Issues related to any of the launchers within SmartSim labels Apr 17, 2024
@ankona ankona marked this pull request as ready for review April 18, 2024 19:25
@codecov Codecov
Copy link

codecov bot commented Apr 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (dragon_launcher@5ef4af5). Click here to learn what that means.

❗ Current head 6732046 differs from pull request most recent head b8611bb. Consider uploading reports for the commit b8611bb to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##             dragon_launcher     #554   +/-   ##
==================================================
  Coverage                   ?   63.95%           
==================================================
  Files                      ?       75           
  Lines                      ?     5440           
  Branches                   ?        0           
==================================================
  Hits                       ?     3479           
  Misses                     ?     1961           
  Partials                   ?        0

@ankona ankona requested a review from al-rigazzi April 22, 2024 15:15
al-rigazzi
al-rigazzi approved these changes Apr 22, 2024
View reviewed changes
Copy link
Collaborator

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

Still LGTM!

@ankona ankona merged commit 9fd7fe6 into CrayLabs:dragon_launcher Apr 23, 2024
33 checks passed
@ankona ankona deleted the 665 branch June 3, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@al-rigazzi al-rigazzi al-rigazzi approved these changes

Assignees
No one assigned
Labels
area: launcher Issues related to any of the launchers within SmartSim type: feature Issues that include feature request or feature idea type: refactor Issues focused on refactoring existing code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ankona @al-rigazzi