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

feat: add cron task that runs the minimal training pipeline nightly #966

Merged
merged 1 commit into from
Jan 6, 2025

Conversation

bhearsum
Copy link
Collaborator

Relates to https://bugzilla.mozilla.org/show_bug.cgi?id=1937882, where we're adding integration tests for the translations pipeline in mozilla-releng/fxci-config#210. These work by looking a decision task and rerunning the tasks it creates (and those upstream of it). Running the pipeline on a cron schedule allows us to have an easily findable, up to date, decision task to point the tests at.

@bhearsum bhearsum requested review from a team as code owners December 18, 2024 18:37
@bhearsum bhearsum requested a review from ahal December 18, 2024 18:37
@bhearsum bhearsum marked this pull request as draft December 19, 2024 01:27
@bhearsum
Copy link
Collaborator Author

It looks like the defaults updating is causing some issues (either with some test assumptions, or genuine issues); moving back into draft while I work that out.

@bhearsum
Copy link
Collaborator Author

It looks like the defaults updating is causing some issues (either with some test assumptions, or genuine issues); moving back into draft while I work that out.

This turns out not to be necessary, so I removed it! (The defaults end up getting set in the parameters:

parameters.setdefault("training_config", {})
.)

This is ready for review now, but it does have #962 cherry picked to it to ensure the correct kind name is used. This isn't landable until that merges and I rebase on top of it.

@bhearsum
Copy link
Collaborator Author

cbc9e4c is the only part that needs review here.

@bhearsum bhearsum marked this pull request as ready for review December 23, 2024 16:29
@bhearsum bhearsum requested a review from a team as a code owner December 23, 2024 16:29
Copy link

@jcristau jcristau left a comment

Choose a reason for hiding this comment

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

Kind of wondering if the cron job shouldn't be using an action instead of decision-task. That'd be a bit more code but wouldn't require the special-casing in get_decision_parameters.

Also, does this really need to run daily (vs weekly for gecko's os-integration cron)?

@bhearsum
Copy link
Collaborator Author

Kind of wondering if the cron job shouldn't be using an action instead of decision-task. That'd be a bit more code but wouldn't require the special-casing in get_decision_parameters.

Hmm...I do recall looking at that, but I'm not sure why that didn't work out. Or I might be confusing that with the abandoned mozilla-releng/fxci-config#192. I'll have a second look at this though; I agree that's cleaner if it works out.

Also, does this really need to run daily (vs weekly for gecko's os-integration cron)?

I think daily is actually more appropriate for this, because all meaningful tasks will be cached 99% of the time. Daily lets us virtually eliminate the possibility that an image change will run against outdated versions of these tasks.

Copy link

@jcristau jcristau left a comment

Choose a reason for hiding this comment

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

r+ to make it clear my comment isn't meant to be blocking.

@bhearsum
Copy link
Collaborator Author

I gave trigger-action another try over here. It reminded me that, unless I'm missing something, it would require duplicating the configuration into .cron.yml or somewhere in fxci-config to have the schema check pass.

@bhearsum bhearsum force-pushed the cron-pipeline branch 2 times, most recently from cbc9e4c to e99cab8 Compare December 27, 2024 15:33
@bhearsum bhearsum merged commit 2fe2ada into mozilla:main Jan 6, 2025
9 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