Skip to content

It adds the ability to cancel multiple jobs #125

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

Merged
merged 1 commit into from
Jul 23, 2019

Conversation

rafaelcapucho
Copy link
Contributor

@rafaelcapucho rafaelcapucho commented Jul 19, 2019

No description provided.

@rafaelcapucho rafaelcapucho changed the title KUMO-3323: It adds the ability to cancel multiple jobs It adds the ability to cancel multiple jobs Jul 19, 2019
Copy link

@ruairif ruairif left a comment

Choose a reason for hiding this comment

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

1 minor change to the logic for validating the input but otherwise it looks good; clear, well commented and documented

👍

@rafaelcapucho rafaelcapucho force-pushed the kumo-3323-cancel-multiple-jobs branch from c14fd4a to 3aac98e Compare July 19, 2019 12:16
@scrapinghub scrapinghub deleted a comment from codecov bot Jul 19, 2019
Copy link
Contributor

@vshlapakov vshlapakov left a comment

Choose a reason for hiding this comment

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

I can only ask for the same minor change mentioned by Ruairi, feel free to add tests. Well done 👍

@rafaelcapucho rafaelcapucho force-pushed the kumo-3323-cancel-multiple-jobs branch from 3aac98e to 943f3f6 Compare July 22, 2019 18:44
@scrapinghub scrapinghub deleted a comment from codecov bot Jul 22, 2019
@rafaelcapucho rafaelcapucho force-pushed the kumo-3323-cancel-multiple-jobs branch from 943f3f6 to aea1b17 Compare July 22, 2019 21:50
@rafaelcapucho
Copy link
Contributor Author

Hello guys @vshlapakov @ruairif ,
The PR is updated, with the suggested changes and the test cases, thx.

@rafaelcapucho rafaelcapucho force-pushed the kumo-3323-cancel-multiple-jobs branch from aea1b17 to 770d39e Compare July 23, 2019 13:54
@codecov
Copy link

codecov bot commented Jul 23, 2019

Codecov Report

Merging #125 into master will decrease coverage by 0.02%.
The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #125      +/-   ##
==========================================
- Coverage   93.61%   93.59%   -0.03%     
==========================================
  Files          28       28              
  Lines        1895     1919      +24     
==========================================
+ Hits         1774     1796      +22     
- Misses        121      123       +2
Impacted Files Coverage Δ
scrapinghub/client/exceptions.py 81.53% <66.66%> (-0.72%) ⬇️
scrapinghub/client/jobs.py 85.71% <95.23%> (+1.68%) ⬆️

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 8319db9...770d39e. Read the comment docs.

@vshlapakov vshlapakov merged commit 3016227 into master Jul 23, 2019
@vshlapakov vshlapakov deleted the kumo-3323-cancel-multiple-jobs branch July 23, 2019 14:31
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.

3 participants