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
Merged

Add concurrent package validation #48

merged 1 commit into from
May 11, 2017

Conversation

willirath
Copy link
Contributor

  • 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
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 ericdill left a comment

Choose a reason for hiding this comment

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

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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])
Copy link
Contributor

Choose a reason for hiding this comment

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

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

@ericdill
Copy link
Contributor

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

@ericdill ericdill merged commit 68b47d2 into vericast:master May 11, 2017
@jhoblitt
Copy link

Outstanding! 👍

@ericdill
Copy link
Contributor

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
Copy link
Contributor

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants