search

fsspec / gdrivefs

Google drive implementation of fsspec
BSD 2-Clause "Simplified" License
38 stars 20 forks source link

Allow travis #5

Closed martindurant closed 5 years ago

martindurant commented 5 years ago

OK, so it seems this would work, but only if we allow PRs only from branches within the repo (i.e., from current collaborators) rather than the usual fork model for anyone. I wonder if this is viable or not.

martindurant commented 5 years ago

Of course, we could defer tests to authors on their own credentials, and then let travis only run tests on the master branch after merge

martindurant commented 5 years ago

@TomAugspurger , to fill you in: this is an attempt to tests the code against real gdrive using real creds, to avoid messing around with vcrpy.

TomAugspurger commented 5 years ago

Does allowing PRs from branches risk exposing credentials?

martindurant commented 5 years ago

It could, so travis forbids it

TomAugspurger commented 5 years ago

I suppose mocking would be quite difficult? I see https://github.com/googleapis/google-api-python-client/blob/master/tests/test_mocks.py, which provides some basics...

martindurant commented 5 years ago

Would involve re-writing the google api in the mock class. Can check for consistency with past code, but not really correct working

TomAugspurger commented 5 years ago

OK. I feel a bit uneasy about adding real user credentials to public CI, but I don't have a strong prior here on what's best.

On Fri, Nov 8, 2019 at 1:46 PM Martin Durant notifications@github.com wrote:

Would involve re-writing the google api in the mock class. Can check for consistency with past code, but not really correct working

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/intake/gdrivefs/pull/5?email_source=notifications&email_token=AAKAOITEGIMW7UY3OIJZNWLQSW6ZHA5CNFSM4JK4OWN2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDTFHTQ#issuecomment-551965646, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKAOIUYWBM6QQ3T6SA7QUDQSW6ZHANCNFSM4JK4OWNQ .

martindurant commented 5 years ago

The travis system is certain meant for this situation, but it has its own pain. I don't want to invoke vcrpy again... So I don't have a good solution either (except: don't use CI!!)

TomAugspurger commented 5 years ago

Given that I'm currently fighting it, I'm +1 to avoiding vcrpy :)