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

Add some logic to pass the correct github ref to mcp script #1990

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

bandish-shah
Copy link
Contributor

@bandish-shah bandish-shah commented Feb 22, 2023

What does this PR do?

Adds logic to the pytest-gpu workflow to pass the correct Github reference to the mcp test runner. If the workflow is triggered by an event in a pull request, it will use the PR number. Otherwise the commit SHA will be used if the run is on a protected branch, else the branch name is used.

Testing

Temporarily added pull_request trigger to kick off the daily workflow in this PR:
https://github.com/mosaicml/composer/actions/runs/4247384552/jobs/7385370010

GPU runs failed due to a failing test in dev so this is expected. Fix for that will be in another PR but should not block this PR.

Before submitting

To Do

  • Remove pull_request trigger that was added to test daily workflow

Copy link
Contributor

@mvpatel2000 mvpatel2000 left a comment

Choose a reason for hiding this comment

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

Approving to unblock -- I assume this will take a few iterations.... Assuming ur testing as much as possible before merging

@bandish-shah
Copy link
Contributor Author

I tested the daily by manually adding the pull_request trigger in the PR. Just removed before merging.

@bandish-shah bandish-shah enabled auto-merge (squash) February 22, 2023 22:50
@bandish-shah bandish-shah merged commit ee925f4 into dev Feb 22, 2023
@bandish-shah bandish-shah deleted the bandish/fix_daily_commit branch February 22, 2023 23:01
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