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

The filter-test-dev CI script doesn't seem to be working #30

Closed
fabianp opened this issue Oct 20, 2023 · 7 comments
Closed

The filter-test-dev CI script doesn't seem to be working #30

fabianp opened this issue Oct 20, 2023 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@fabianp
Copy link
Contributor

fabianp commented Oct 20, 2023

see for example https://github.com/iclr-blogposts/2024/actions/runs/6591771757

I'm commenting out for now the offending lines since the upload script depends on the success of this script

@fabianp fabianp added the bug Something isn't working label Oct 20, 2023
@fabianp
Copy link
Contributor Author

fabianp commented Oct 20, 2023

I suspect that's because of some security restrictions of github actions. It might be necessary to put some of those tests in a different workflow_run file as I've done in deploy-for-review.yml. See for example https://stackoverflow.com/a/71683208

@Velythyl
Copy link
Contributor

Alright so I did a LOT of new commits to fix the scripts for when an external PR is made.

I found some issues:

  1. When an external person makes a PR, we have to manually "Approve & Run" the actions for it. This is doable manually but very annoying. -> TODO
  2. When the filters fail, the comment wouldn't get written at all. This would also cause the S3 upload to bail prematurely. -> [FIXED in both cases]

@Velythyl
Copy link
Contributor

The new file for writing comments on filter failures is here: https://github.com/iclr-blogposts/2024/blob/main/.github/workflows/comment-on-error.yaml

Here is an example PR from an external account that. You can tell by the automated comments on that PR that at first, I tested a success case, and then I swapped to a failure case. #52

@fabianp
Copy link
Contributor Author

fabianp commented Nov 21, 2023

Regarding your first point, I believe we have to manually approve it only if it's a new github account (I don't know how github determines exactly that an account is new)

@fabianp
Copy link
Contributor Author

fabianp commented Nov 21, 2023

thanks BTW for the work on fixing the actions!

@Velythyl
Copy link
Contributor

I believe we have to manually approve it only if it's a new github account

ah ok I see! Maybe we can add a small message in the PR template to mention that if their run doesn't start automatically, they have to @ us to approve it

@fabianp
Copy link
Contributor Author

fabianp commented Nov 22, 2023

yes that seems like a good idea

@fabianp fabianp closed this as completed Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants