Skip to content

More robust gen_protos on jenkins.#3224

Closed
robertwb wants to merge 2 commits into
apache:masterfrom
robertwb:gen-proto
Closed

More robust gen_protos on jenkins.#3224
robertwb wants to merge 2 commits into
apache:masterfrom
robertwb:gen-proto

Conversation

@robertwb
Copy link
Copy Markdown
Contributor

Don't use the globally shared build directory, which pip may refuse
to use if there's a failed install.

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

  • Make sure the PR title is formatted like:
    [BEAM-<Jira issue #>] Description of pull request
  • Make sure tests pass via mvn clean verify.
  • Replace <Jira issue #> in the title with the actual Jira issue
    number, if there is one.
  • If this contribution is large, please file an Apache
    Individual Contributor License Agreement.

Don't use the globally shared build directory, which pip may refuse
to use if there's a failed install.
@robertwb
Copy link
Copy Markdown
Contributor Author

R: @aaltay

Copy link
Copy Markdown
Member

@aaltay aaltay left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment thread sdks/python/gen_protos.py Outdated
build_path = install_path + '-build'
if os.path.exists(build_path):
shutil.rmtree(build_path)
logging.warning('Downloading a grpcio-tools to %s' % install_path)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Downloading -> Installing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread sdks/python/gen_protos.py Outdated
logging.warning('Downloading a grpcio-tools to %s' % install_path)
subprocess.check_call(
['pip', 'install', '-t', install_path, '--upgrade', GRPC_TOOLS])
['pip', 'install', '-t', install_path, '-b', build_path,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

-t -> --target

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.01%) to 70.716% when pulling 662414a on robertwb:gen-proto into 3643676 on apache:master.

@robertwb
Copy link
Copy Markdown
Contributor Author

Thanks.

@asfgit asfgit closed this in a03a638 May 25, 2017
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 70.702% when pulling c74b608 on robertwb:gen-proto into 3643676 on apache:master.

@jkff
Copy link
Copy Markdown
Contributor

jkff commented May 25, 2017

This PR seems to greatly slow down mvn clean compile because now instead of just compiling Java code, this also installs some python packages and compiles a ton of C code.

Can still be worked around by adding -pl '!sdks/python', which brings time for mvn clean compile test-compile from 5:30 minutes to 1:30.

@robertwb
Copy link
Copy Markdown
Contributor Author

robertwb commented May 25, 2017 via email

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