Skip to content

[CI] Drop permissions: from AWS CUDA precommit task #13674

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

Closed
wants to merge 1 commit into from

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