Skip to content

Conversation

@aelovikov-intel
Copy link
Contributor

It's been broken since #13173.

@aelovikov-intel aelovikov-intel requested a review from a team as a code owner May 6, 2024 22:57
@aelovikov-intel
Copy link
Contributor Author

I expect @intel/llvm-reviewers-cuda to follow up and find out what particular permissions are required for the task to work. Restore to unrestricted for now.

@npmiller
Copy link
Contributor

npmiller commented May 7, 2024

Unfortunately that doesn't fix it (I've also tried it here, and @jsji tried a few different things here):

That's why we were suspecting there's other permission configuration issue but we're not able to look at that on our side.

@npmiller
Copy link
Contributor

npmiller commented May 7, 2024

Ooooh of course, scratch that, I just realized the changes in the PRs aren't getting used by the pre-commit, is there any way to test this before merging, so we can investigate the exact permissions?

@aelovikov-intel
Copy link
Contributor Author

is there any way to test this before merging, so we can investigate the exact permissions?

No, I don't think so.

@jsji
Copy link
Contributor

jsji commented May 7, 2024

is there any way to test this before merging, so we can investigate the exact permissions?

No, I don't think so.

If the pre-commit test won't be able to test the logic, would it make sense for us to try merging #13377 first before this?

If 13377 doesn't work after merge, then we can revert it and merge this?

@aelovikov-intel
Copy link
Contributor Author

is there any way to test this before merging, so we can investigate the exact permissions?

No, I don't think so.

If the pre-commit test won't be able to test the logic, would it make sense for us to try merging #13377 first before this?

If 13377 doesn't work after merge, then we can revert it and merge this?

Yes! Can you re-open it?

@jsji
Copy link
Contributor

jsji commented May 7, 2024

Yes! Can you re-open it?

Great. Re-opened. #13377

@aelovikov-intel aelovikov-intel deleted the restore-aws-e2e branch May 7, 2024 16:52
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.

3 participants