Skip to content

Conversation

@Borda
Copy link
Collaborator

@Borda Borda commented May 22, 2020

What does this PR do?

remove default list which can be dangerous usage

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@Borda Borda added the bug Something isn't working label May 22, 2020
@Borda Borda added this to the 0.7.7 milestone May 22, 2020
@mergify mergify bot requested a review from a team May 22, 2020 19:30
@Borda Borda marked this pull request as ready for review May 25, 2020 08:59
@mergify
Copy link
Contributor

mergify bot commented May 25, 2020

This pull request is now in conflict... :(

@awaelchli
Copy link
Contributor

is the retry_jittered_backoff not working well? I thought this was a feature.

@codecov
Copy link

codecov bot commented May 25, 2020

Codecov Report

Merging #1927 into master will increase coverage by 0%.
The diff coverage is 88%.

@@          Coverage Diff           @@
##           master   #1927   +/-   ##
======================================
  Coverage      88%     88%           
======================================
  Files          74      74           
  Lines        4621    4607   -14     
======================================
- Hits         4050    4048    -2     
+ Misses        571     559   -12     

@Borda Borda requested a review from justusschock May 25, 2020 09:24
@Borda
Copy link
Collaborator Author

Borda commented May 25, 2020

is the retry_jittered_backoff not working well? I thought this was a feature.

It was not used anywhere...



def pick_single_gpu(exclude_gpus=[]):
# def retry_jittered_backoff(f, num_retries=5):
Copy link
Contributor

Choose a reason for hiding this comment

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

@Borda ?

Copy link
Collaborator Author

@Borda Borda May 25, 2020

Choose a reason for hiding this comment

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

is this used somewhere?
EDIT: ok, uncommented...

@mergify mergify bot requested a review from a team May 25, 2020 11:46
@pep8speaks
Copy link

pep8speaks commented May 25, 2020

Hello @Borda! Thanks for updating this PR.

Line 751:111: E501 line too long (115 > 110 characters)

Comment last updated at 2020-05-26 16:08:38 UTC

@Borda Borda requested a review from williamFalcon May 25, 2020 12:51
@Borda Borda added the ready PRs ready to be merged label May 25, 2020
@Borda Borda modified the milestones: 0.7.7, 0.8.0 May 26, 2020
@williamFalcon williamFalcon merged commit 5e8c5ab into master May 26, 2020
@Borda Borda deleted the fix/default branch May 27, 2020 08:21
justusschock pushed a commit that referenced this pull request Jun 29, 2020
* fix default

* formatting errors

* update

* flake8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants