Skip to content
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

use 2to3 to convert from Python2 to 3 and use yapf to reformat #35

Closed
wants to merge 31 commits into from

Conversation

jimexist
Copy link
Member

@jimexist jimexist commented Feb 15, 2018

low priority

use:

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@jimexist
Copy link
Member Author

jimexist commented Feb 15, 2018

the benefit / cost for this PR is a bit low so it is really low priority for now. I am just submitting for view only.

@jlewi
Copy link
Contributor

jlewi commented Feb 15, 2018

Please sync. Can you pick a reviewer (other than me :)) I'd like to start spreading the review workload around.

@jimexist
Copy link
Member Author

to be merged after #36

@jimexist
Copy link
Member Author

/assign @gaocegege
/assign @DjangoPeng
/assign @ScorpioCPH

@jimexist jimexist changed the title low priority, use 2to3 and yapf use 2to3 to convert from Python2 to 3 and use yapf to reformat Feb 28, 2018
@jimexist
Copy link
Member Author

seems like the presubmit job is a bit unstable for now

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: djangopeng

Assign the PR to them by writing /assign @djangopeng in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jimexist
Copy link
Member Author

jimexist commented Mar 1, 2018

@jlewi any idea why the link gave by bot was invalid?

@jlewi
Copy link
Contributor

jlewi commented Mar 1, 2018

@jimexist If you see that error

Unable to load build details from gs://kubernetes-jenkins/pr-logs/pull/kubeflow_testing/35/kubeflow-testing-presubmit/56

It typically means the prow job didn't make it as far as actually uploading any results to GCS.

So typically in this case you can trigger the test and then look for the job in the prow job dashboard. From there you can grab the actual pod logs. These get garbage collected pretty soon after the job finishes so you need to try to catch it in the act.

@jlewi
Copy link
Contributor

jlewi commented Mar 1, 2018

Here's the link to the prow jobs dashboard
https://prow.k8s.io/?repo=kubeflow%2Ftesting

@jlewi
Copy link
Contributor

jlewi commented Mar 1, 2018

/test all

1 similar comment
@jlewi
Copy link
Contributor

jlewi commented Mar 2, 2018

/test all

@jlewi
Copy link
Contributor

jlewi commented Mar 3, 2018

/test all

1 similar comment
@jlewi
Copy link
Contributor

jlewi commented Mar 3, 2018

/test all

@jlewi
Copy link
Contributor

jlewi commented Mar 20, 2018

Most likely that means there is a problem running the workflow and it never gets far enough to upload the results to GCS.

You need to try to look at the pod logs for the test; these are ephmeral so you need to grab them shortly after running your job.

You can find them here
https://prow.k8s.io/?repo=kubeflow%2Ftesting

@jimexist
Copy link
Member Author

jimexist commented Mar 21, 2018

@jlewi maybe due to lack of authorization, what i saw in https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/kubeflow_testing/35/kubeflow-testing-presubmit/99/ was:

Unable to load build details from gs://kubernetes-jenkins/pr-logs/pull/kubeflow_testing/35/kubeflow-testing-presubmit/99

@jlewi
Copy link
Contributor

jlewi commented Mar 21, 2018

I was able to get access to the logs. Here's the failure

"Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/src/kubeflow/testing/py/kubeflow/testing/run_e2e_workflow.py", line 36, in <module>
    from kubeflow.testing import argo_client
  File "/src/kubeflow/testing/py/kubeflow/testing/argo_client.py", line 9, in <module>
    from kubeflow.testing import util
  File "/src/kubeflow/testing/py/kubeflow/testing/util.py", line 10, in <module>
    import urllib.request, urllib.parse, urllib.error
ImportError: No module named request

@jimexist
Copy link
Member Author

/retest

@k8s-ci-robot
Copy link
Contributor

@jimexist: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
kubeflow-testing-presubmit 4112bf7 link /test kubeflow-testing-presubmit

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jimexist
Copy link
Member Author

see #86 in leu of

@jimexist jimexist closed this Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify python code is formatted with yapf
8 participants