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

Adding perf testing GHA #5851

Merged
merged 15 commits into from
May 5, 2023
Merged

Adding perf testing GHA #5851

merged 15 commits into from
May 5, 2023

Conversation

leahwicz
Copy link
Contributor

@leahwicz leahwicz commented Sep 16, 2022

resolves #4021

Description

This PR creates an Action that automates running our perf regression testing framework after a release. It also reports those results and creates a PR for them so we can continue to keep track of the numbers over time

I pulled this work out of this draft PR #4872 and just targeted the single action to gather results after a release is made (the other action is out of scope for this)

Example PRs created here

Checklist

@cla-bot cla-bot bot added the cla:yes label Sep 16, 2022
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@leahwicz leahwicz added the Skip Changelog Skips GHA to check for changelog file label Sep 16, 2022
@leahwicz leahwicz marked this pull request as ready for review September 16, 2022 14:31
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Why do we only run against main? When we release 1.2.7 it could have significantly diverged from main.

@leahwicz
Copy link
Contributor Author

Why do we only run against main?
@nathaniel-may would you want to field this question?

@nathaniel-may
Copy link
Contributor

the model action should be pulling the action code from main always, but checking out dbt code from whatever branch we want to model.

.github/workflows/model_performance.yml Outdated Show resolved Hide resolved
.github/workflows/model_performance.yml Outdated Show resolved Hide resolved
.github/workflows/model_performance.yml Outdated Show resolved Hide resolved
.github/workflows/model_performance.yml Outdated Show resolved Hide resolved
.github/workflows/model_performance.yml Show resolved Hide resolved

# `${{ github.workspace }}` is used to pass the absolute path
# TODO CHANGE NUMBER OF RUNS BEFORE MERGING
# TODO this isn't putting the baseline in the right directory. it's putting it one level up.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nathaniel-may do you remember what this comment meant? It looks like the PR shows the file is in the right directory

@leahwicz leahwicz requested a review from emmyoop September 22, 2022 16:27
@leahwicz
Copy link
Contributor Author

@emmyoop @nathaniel-may could I get a look at this again please? I do want to get these times documented for all the 1.0+ versions as we get further away from 1.0

Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Just updating to use the new env variable for outputs everywhere but nothing else jumps out.

.github/workflows/model_performance.yml Outdated Show resolved Hide resolved
.github/workflows/model_performance.yml Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

This PR has been marked as Stale because it has been open with no activity as of late. If you would like the PR to remain open, please comment on the PR or else it will be closed in 7 days.

@github-actions github-actions bot added the stale Issues that have gone stale label Apr 30, 2023
@leahwicz leahwicz removed the stale Issues that have gone stale label May 2, 2023
@leahwicz leahwicz requested a review from emmyoop May 3, 2023 02:33
Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

Looks good to me. There are several "remove/change before merging" comments to take care of before merging.

@leahwicz leahwicz closed this May 4, 2023
@leahwicz leahwicz reopened this May 4, 2023
@leahwicz leahwicz requested a review from a team as a code owner May 4, 2023 18:29
@leahwicz leahwicz requested review from davidbloss and removed request for a team May 4, 2023 18:29
@leahwicz leahwicz merged commit 810ef7f into main May 5, 2023
@leahwicz leahwicz deleted the leahwicz/perfTesting branch May 5, 2023 13:41
@aranke aranke mentioned this pull request May 9, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes Skip Changelog Skips GHA to check for changelog file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-14] Enable Regression Testing Notifications
3 participants