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

Fix run_e2e_workflow #33

Merged
merged 18 commits into from
Feb 15, 2018
Merged

Fix run_e2e_workflow #33

merged 18 commits into from
Feb 15, 2018

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Feb 15, 2018

  • run_e2e_workflow.py is exiting with an exception causing all prow jobs to be marked as a failure

  • Add an E2E test to kubeflow/testing. This provides a way to test run_e2e_workflow.py as part of the presubmits.

Fix #30

@jlewi jlewi changed the title Add E2E test. Fix run_e2e_workflow Feb 15, 2018
@jlewi jlewi requested a review from jimexist February 15, 2018 14:16

time.sleep(polling_interval.seconds)

return []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new line after this pls

"vendor",]
full_dir_excludes = [os.path.join(os.path.abspath(args.src_dir), f) for f in
dir_excludes]
includes = ["*.py"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would really benefit from Python3's Pathlib

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack.

topdown=True):
# excludes can be done with fnmatch.filter and complementary set,
# but it's more annoying to read.
exclude = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be extracted into a smaller functions:

def should_exclude(root, full_dir_excludes):
  for e in full_dir_excludes:
    if root.startswith(e):
      return True
  else:
    return False

@jimexist
Copy link
Member

/lgtm

@k8s-ci-robot
Copy link
Contributor

@jimexist: changing LGTM is restricted to assignees, and assigning you to the PR failed.

In response to this:

/lgtm

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.

@jlewi
Copy link
Contributor Author

jlewi commented Feb 15, 2018

/test all

Copy link
Member

@pdmack pdmack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@jlewi
Copy link
Contributor Author

jlewi commented Feb 15, 2018

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.

4 participants