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

Add concurrent package validation #48

Merged
merged 1 commit into from May 11, 2017

Conversation

@willirath
Copy link
Contributor

willirath commented May 11, 2017

  • Use multiprocessing from the std lib to concurrently validate
    packages. Add a cli argument num-threads setting the number of
    concurrent processes to be used.

  • Add test for three concurrent scenarios: num_threads=0 using all
    available cores, num_threads=1 for serial validation,
    num_threads=4 explicitly using four processes.

  • Adapt Travis script to properly measure test coverage with concurrent
    validation.

This is a cleaned-up version of #45.

- Use `multiprocessing` from the std lib to concurrently validate
  packages.  Add a cli argument `num-threads` setting the number of
  concurrent processes to be used.

- Add test for three concurrent scenarios:  `num_threads=0` using all
  available cores, `num_threads=1` for serial validation,
  `num_threads=4` explicitly using four processes.

- Adapt travis script properly measure test coverage with concurrent
  validation.
@codecov

This comment has been minimized.

Copy link

codecov bot commented May 11, 2017

Codecov Report

Merging #48 into master will increase coverage by 0.47%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #48      +/-   ##
==========================================
+ Coverage   94.06%   94.53%   +0.47%     
==========================================
  Files           2        2              
  Lines         219      238      +19     
==========================================
+ Hits          206      225      +19     
  Misses         13       13
Impacted Files Coverage Δ
conda_mirror/conda_mirror.py 94.46% <100%> (+0.48%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f352883...0084a07. Read the comment docs.

Copy link
Contributor

ericdill left a comment

LGTM. Thanks for the effort on this! Testing this branch locally and will report back after it's complete. If everything goes smoothly, i'll merge this, tag a new release and upload a new version to pypi/conda-forge

are not in `repodata` and also any packages that fail the package
validation
NOTE2: In concurrent mode (num_threads is not 1) this might be hard to kill
using CTRL-C.

This comment has been minimized.

Copy link
@ericdill

ericdill May 11, 2017

Contributor

it's actually pretty simple to kill with ctrl+c. I just hit ctrl+c a few times and everything died appropriately ¯_(ツ)_/¯

@@ -456,7 +521,8 @@ def main(upstream_channel, target_directory, temp_directory, platform,
# 1. validate local repo
# validating all packages is taking many hours.
# _validate_packages(repodata=repodata,
# package_directory=local_directory)
# package_directory=local_directory,
# num_threads=num_threads)

This comment has been minimized.

Copy link
@ericdill

ericdill May 11, 2017

Contributor

Shame on me for leaving commented code in this project.

👍 to you for updating commented code with the new API 😀

@@ -41,7 +41,8 @@ def test_version():
@pytest.mark.parametrize(
'channel,platform',
itertools.product([anaconda_channel, 'conda-forge'], ['linux-64']))
def test_cli(tmpdir, channel, platform, repodata):
@pytest.mark.parametrize('num_threads', [0, 1, 4])

This comment has been minimized.

Copy link
@ericdill

ericdill May 11, 2017

Contributor

Did not know you could combine multiple parametrize decorators. Neat!

@ericdill

This comment has been minimized.

Copy link
Contributor

ericdill commented May 11, 2017

works locally. Thanks @willirath ! Merging, tagging and deploying a new version to pypi/conda-forge

@ericdill ericdill merged commit 68b47d2 into Valassis-Digital-Media:master May 11, 2017
3 checks passed
3 checks passed
codecov/patch 100% of diff hit (target 94.06%)
Details
codecov/project 94.53% (+0.47%) compared to f352883
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jhoblitt

This comment has been minimized.

Copy link

jhoblitt commented May 11, 2017

Outstanding! 👍

@ericdill

This comment has been minimized.

Copy link
Contributor

ericdill commented May 11, 2017

should be able to pip install 0.7.0 which has the additional command line flag --num-threads that allows you to do concurrent package validation.

@ericdill

This comment has been minimized.

Copy link
Contributor

ericdill commented May 11, 2017

conda package for 0.7.0 should be available in the next week or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.