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

Advice on handling scheduled event builds #100

Open
joehorsnell opened this issue Aug 16, 2021 · 11 comments
Open

Advice on handling scheduled event builds #100

joehorsnell opened this issue Aug 16, 2021 · 11 comments

Comments

@joehorsnell
Copy link

Hi @dorny,

We have a workflow containing multiple triggers:

  • push events (to the default branch)
  • pull requests (against the default branch)
  • workflow dispatch (i.e. manually-triggered)
  • scheduled events (i.e. cron)

Your paths-filter action works great for the first three use-cases, detecting the context and base to compare against. However, it fails for scheduled events giving the error This action requires 'base' input to be configured or 'repository.default_branch' to be set in the event payload.

Looking in your code at where the error is thrown, we could set a base, but I'm not sure how to set that in a way that would work for the other triggers.

Any suggestions on how to handle this? For a scheduled build there is obviously no "base" to compare against as it's just a specific commit that is used, but ideally it would fall-back and handle this situation without raising an error?

Thanks,

Joe.

@joehorsnell
Copy link
Author

PS. Thanks for building this GH Action, it's great 👍.

@dorny
Copy link
Owner

dorny commented Aug 27, 2021

Hi @joehorsnell,

What exactly do you expect from handling the situation without raising the error?
Do you want to run the whole build, like everything was changed?
Or do you want to somehow detect changes since the previously scheduled build?
And thanks for reporting the issue. I didn't think about this use-case before 👍

@joehorsnell
Copy link
Author

Hi @dorny, thanks for getting back to me.

I think ideally it would just use the latest commit on whatever branch the scheduled build was triggered for, much like for a build on the default branch (main/master)?

@dorny
Copy link
Owner

dorny commented Sep 2, 2021

much like for a build on the default branch
If you mean build after push to the branch, then there we have available commit hash of the last previously pushed commit.
For scheduled events, it doesn't make sense. Detecting changes solely from the last commit won't be much useful too.

Assuming you run the build on a daily basis, it would actually make sense to compare the current version against the last commit from the past day.

For example, you could find the commit ID like this:

git log --before `date --iso-8601`T00:00:00 -n 1 --pretty=format:"%h"

This is something I would prefer to not include directly in this action.
You could pack the code as a separate action or simply run it as a previous step and store the commit ID in a variable.
Then use the commit ID as a value for the base input parameter.

@joehorsnell
Copy link
Author

Assuming you run the build on a daily basis, it would actually make sense to compare the current version against the last commit from the past day.

I'm not sure that behaviour is too intuitive to be honest - the schedule is defined by a cron, which could be any frequency at all. There is nothing to accurately tie the time a build runs with a particular commit (i.e. the cron schedule is a request to run at a certain time, but it could be delayed, as documented here).

What I'm suggesting is just having a sensible default for scheduled builds, so that rather than causing an error, it works in the same way as a build for a branch push does - i.e. take the latest commit for the branch that the build is running for and use that to calculate the diff?

Is there a way I can set that myself just for scheduled builds, without breaking the existing behaviour for the other triggers?

Thanks!

@jlarmstrongiv
Copy link

I had this error too. How would I always consider scheduled builds as all new?

joehorsnell added a commit to joehorsnell/paths-filter that referenced this issue Jan 27, 2022
Related to dorny#100 (can't say it fixes it, but at least clarifies usage), this PR adds a section to the
docs to explain how to handle using this action in a workflow triggered by a [`schedule` event][1].

[1]: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
@joehorsnell
Copy link
Author

I had this error too. How would I always consider scheduled builds as all new?

@jlarmstrongiv - FYI I opened #120 to update the docs and clarify usage around scheduled events.

joehorsnell added a commit to joehorsnell/paths-filter that referenced this issue Feb 28, 2022
Related to dorny#100 (can't say it fixes it, but at least clarifies usage), this PR adds a section to the
docs to explain how to handle using this action in a workflow triggered by a [`schedule` event][1].

[1]: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule
@davinchia
Copy link

We are trying to do that at https://github.com/airbytehq/airbyte and also running into this issue. It'll be great if this action can detect the scheduled event and default to the last commit on master.

@dorny
Copy link
Owner

dorny commented Jun 13, 2022

Sorry for the delay, I've been quite busy the last few months.

IMHO, there's not much use for this action with scheduled events. At the same time, it should not stay in the way and complicate the workflow definitions more than necessary. So, I've come to the conclusion:

  • By default, all files will be considered as changed - this will allow using the same job definition for scheduled events without any ifs. The most common usage of this action is for running tests, so running everything by default seems like a sensible default.
  • If base is specified:
    • if is different than the current branch, it will be used to do the comparison as usual
    • if it's the same as the current branch, the changes from the last commit will be used

This way it should support both behaviors that were proposed in this issue. I will make it part of the next release later this month or so. I haven't gone through all the reported issues yet, so depends on what I will find there.

@joehorsnell
Copy link
Author

Hi @dorny - yep, agreed, I think the best thing is for this action to just consider everything as having changed for a scheduled event. If people want specific behaviour for scheduled events, they can always do that, but the main thing is just having it not cause an error on that event type.

@jakemac53
Copy link

jakemac53 commented May 26, 2023

I would also like this behavior where the scheduled jobs just run everything. Basically the goal of those is to ensure everything is still working with all of our newest external dependencies. So we want to run all jobs.

I would also like an option to run all jobs for pushes, but not PRs. So the PR workflow is faster but all pushes run all tests. Is that something that is possible to accomplish today?

I don't know if this could just be generalized so you could list the event types for which you want all outputs to be true?

Edit: Actually I realized I could probably just customize my if to check for these - its a bit more boilerplate but should work fine.

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

No branches or pull requests

5 participants