-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: Enable Google BigQuery (pandas.io.gbq) integration testing #11089 #14111
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
Conversation
whoo hoo! only thing, the attached encrypted json file is decrpyted by the obfuscated travis variables right? IOW it is ONLY usable on travis |
Correct! The json credentials file should only be decrypted on Travis. The private key is tied to the repository and only available in the Travis environment. |
@jreback I'm not sure if I have access to encrypt the file for the pydata/pandas repo (I used my forked repo). Are you able to run You'll need to rename the credentials file (previously sent via email) as Also, please modify line 232 in Here is the error I get when I try to create
I didn't receive this error on my forked repo. It may work if you run it |
did a PR to your branch. I moved the files to ci/ but the keys should work. lmk if you need anything. |
6df7f6e
to
89e7091
Compare
I used the changes from parthea#1 in this PR and I receive the following error:
I believe there is a security restriction on pull requests from forked repos as mentioned here:
Can I show a successful pass on my forked repo with my own encrypted file ? |
Are these errors expected?
|
u need to push tags to your master branch |
@jreback Thanks! That was it. All tests pass in this PR from my forked repo. Only the first commit (89e709154b0417df163c93d33c3f3aecea624551) should be merged . The second commit contains a credentials file which was encrypted from my forked repo (55821a65da69193e8e471944e9b0074474b290d8) and won't be required. If you want to see that Travis is green prior to merging, one solution is to create a branch from pydata/pandas and merge commit (89e709154b0417df163c93d33c3f3aecea624551) into that branch. I expect the correct (pydata/pandas) credentials will work since the branch is not from a different(forked) repo. |
ok the first commit contains the correct credentials? |
Yes. The first commit contains the correct credentials from parthea#1 Only merge the first commit. If its not too much trouble, you could create a PR from pydata/pandas and merge only the first commit just to make sure. The credentials should work since the PR branch is not from a forked repo. |
np I am going to (later) push this up as a PR on master itself so should build |
the only thing is we may need to put in a check to skip trying to get these credentials when not on the master branch |
55821a6
to
c0543c7
Compare
c0543c7
to
1a13487
Compare
Current coverage is 85.27% (diff: 100%)@@ master #14111 diff @@
==========================================
Files 139 139
Lines 50511 50511
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 43071 43071
Misses 7440 7440
Partials 0 0
|
Done Tests are skipped when the encryption key is not available on Travis. See https://travis-ci.org/pydata/pandas/builds/156106141
|
@parthea ok I pushed up that 1st commit. pls check out the build and see if it works. ideally we could have some instructions for users to test on their forks as well ....any ideas? |
@parthea I am not sure this worked: https://travis-ci.org/pydata/pandas/jobs/156506548 |
It appears to be working. The following 2 tests are supposed to be skipped:
The first test only works with a Google Environment, and the 2nd test can't be automated (we use a service account). I'll look into adding a PR to update the contributing documentation. |
ahh ok great!. So at the very least a change that doesn't pass the integration tests will at least fail master!. great! |
I've added instructions for users to run Google BigQuery integration tests on their forks in #14144 |
git diff upstream/master | flake8 --diff