-
Notifications
You must be signed in to change notification settings - Fork 111
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
Fix Actions to skip S3 tests from external contributors #27
Comments
This would be helpful! Also, If S3 access is crucial even for outside cotnributors there are some world-readable-free aws buckets such as PMC Open Access (OA) Subset on AWS |
That's an interesting idea! Some of the tests require buckets to be
writable, so we need to skip some anyway, but it would be nice to keep at
least some of the tests.
…On Aug 21, 2023 at 06:06:40, Chris Ha ***@***.***> wrote:
This would be helpful!
Some reproducible and accessible data, even if it requires some work on
the external collaborators part, could be a reasonable alternative as well.
Also, If S3 access is crucial even for outside cotnributors there are some
world-readable-free aws buckets such as PMC Open Access (OA) Subset on AWS
<https://www.ncbi.nlm.nih.gov/pmc/tools/pmcaws/> s3://pmc-oa-opendata
I am not sure if it is acceptable access to use it for this purpose. (If I
had to guess it would be no)
—
Reply to this email directly, view it on GitHub
<#27 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA53OIQXNQHPKEQJTG6HBJDXWNMOBANCNFSM6AAAAAA3XSWFBM>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
Also is this issue that is blocking the assessment and merging of those two PRs at this time? |
@chris-ha458 I think the fixed I just merged in main should do it. if you have time to rebase, it would be great! |
ok looks like it worked for #24 ! closing this issue. |
This would make the failures for #23 and #24 go away.
Having external collaborators hit the test bucket is a security risk, so those tests should be skipped.
The text was updated successfully, but these errors were encountered: