Skip to content
This repository has been archived by the owner on Sep 13, 2023. It is now read-only.

run heroku tests on detection from title OR schedule #433

Merged
merged 4 commits into from
Oct 28, 2022

Conversation

madhur-tandon
Copy link
Contributor

No description provided.

@madhur-tandon madhur-tandon requested a review from a team October 7, 2022 13:57
@madhur-tandon madhur-tandon temporarily deployed to internal October 7, 2022 13:57 Inactive
@madhur-tandon madhur-tandon temporarily deployed to internal October 7, 2022 13:59 Inactive
@madhur-tandon madhur-tandon temporarily deployed to internal October 7, 2022 14:03 Inactive
@madhur-tandon madhur-tandon temporarily deployed to internal October 7, 2022 14:07 Inactive
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Base: 87.81% // Head: 84.81% // Decreases project coverage by -2.99% ⚠️

Coverage data is based on head (03e92cf) compared to base (663862f).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #433      +/-   ##
==========================================
- Coverage   87.81%   84.81%   -3.00%     
==========================================
  Files          96       96              
  Lines        7909     7804     -105     
==========================================
- Hits         6945     6619     -326     
- Misses        964     1185     +221     
Impacted Files Coverage Δ
mlem/contrib/rabbitmq.py 48.05% <0.00%> (-50.67%) ⬇️
mlem/contrib/kubernetes/utils.py 21.05% <0.00%> (-42.11%) ⬇️
mlem/contrib/kubernetes/base.py 44.08% <0.00%> (-39.79%) ⬇️
mlem/contrib/docker/base.py 57.50% <0.00%> (-30.13%) ⬇️
mlem/contrib/kubernetes/build.py 70.00% <0.00%> (-30.00%) ⬇️
mlem/contrib/docker/utils.py 58.82% <0.00%> (-24.68%) ⬇️
mlem/contrib/kubernetes/service.py 41.37% <0.00%> (-10.35%) ⬇️
mlem/utils/fslock.py 88.05% <0.00%> (-3.49%) ⬇️
mlem/contrib/venv.py 86.13% <0.00%> (-2.98%) ⬇️
mlem/cli/dev.py 47.36% <0.00%> (-2.64%) ⬇️
... and 34 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@madhur-tandon madhur-tandon temporarily deployed to internal October 10, 2022 05:30 Inactive
@madhur-tandon madhur-tandon self-assigned this Oct 10, 2022
@aguschin aguschin temporarily deployed to internal October 11, 2022 13:43 Inactive
Copy link
Contributor

@aguschin aguschin left a comment

Choose a reason for hiding this comment

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

Good work! Trying to understand how this should work. Will it require commit hash when one tries to run the tests manually? Also, can I test it out in this branch before merging to main?

UPD: I just did a commit assuming it will ask me for input (commit hash), but looks like this works differently.

@aguschin
Copy link
Contributor

@madhur-tandon could use your help here to understand this ^ :)

@aguschin aguschin added the 🏡 housekeeping Fighting technical debt and improving dev process label Oct 17, 2022
@madhur-tandon madhur-tandon temporarily deployed to internal October 27, 2022 19:57 Inactive
@madhur-tandon
Copy link
Contributor Author

@madhur-tandon could use your help here to understand this ^ :)

Earlier, I was using something known as a workflow dispatch -- i.e. you need to trigger an action manually.
But now, I have switched to detection of the keyword heroku in title of the PR

@madhur-tandon
Copy link
Contributor Author

The PRs #457 and #458 confirm that this works 😄

@madhur-tandon madhur-tandon changed the title run heroku tests on schedule + trigger from hash run heroku tests on detection from title Oct 27, 2022
@aguschin
Copy link
Contributor

Looking good! Thanks @madhur-tandon!

I remember we had a discussion about doing the same for k8s/sagemaker. Don't remember the resolution. Should we do this for them also?

@madhur-tandon madhur-tandon temporarily deployed to internal October 28, 2022 06:21 Inactive
@madhur-tandon madhur-tandon changed the title run heroku tests on detection from title run heroku tests on detection from title OR schedule Oct 28, 2022
@madhur-tandon
Copy link
Contributor Author

madhur-tandon commented Oct 28, 2022

I remember we had a discussion about doing the same for k8s/sagemaker. Don't remember the resolution. Should we do this for them also?

K8s test are self contained, won't be necessary.
@mike0sv can comment on Sagemaker maybe.

@aguschin
Copy link
Contributor

K8s test are self contained, won't be necessary.

how much time do they take? Speeding up CI would be a good thing also.

@madhur-tandon
Copy link
Contributor Author

how much time do they take? Speeding up CI would be a good thing also.

The point of this PR was not to speed up CI, it was to control flaky behaviour of heroku so we don't get unnecessary failures.

@madhur-tandon madhur-tandon temporarily deployed to internal October 28, 2022 13:32 Inactive
@madhur-tandon madhur-tandon merged commit c720634 into main Oct 28, 2022
@madhur-tandon madhur-tandon deleted the heroku-scheduled branch October 28, 2022 13:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🏡 housekeeping Fighting technical debt and improving dev process
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants