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

[bufix] Batch submit jobs needs to filter the input keyword arguments #14

Merged
merged 2 commits into from
Jun 11, 2020

Conversation

nh13
Copy link
Member

@nh13 nh13 commented Jun 11, 2020

If we have arguments that are None, then we have not include them as keyword
arguments.

See: https://github.com/vemel/mypy_boto3_builder/issues/30
See: boto/botocore#2075

If we have arguments that are None, then we have **not** include them as keyword
arguments.

See: https://github.com/vemel/mypy_boto3_builder/issues/30
See: boto/botocore#2075
@nh13 nh13 requested a review from sstadick June 11, 2020 19:26
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2020

Codecov Report

Merging #14 into master will increase coverage by 3.9%.
The diff coverage is 78.2%.

@@           Coverage Diff            @@
##           master     #14     +/-   ##
========================================
+ Coverage    73.3%   77.3%   +3.9%     
========================================
  Files          11      11             
  Lines         428     441     +13     
  Branches       52      45      -7     
========================================
+ Hits          314     341     +27     
+ Misses         94      89      -5     
+ Partials       20      11      -9     

@@ -244,17 +260,37 @@ def stream(self) -> Optional[str]:

def submit(self) -> SubmitJobResponseTypeDef:
"""Submits this job."""

# If we have arguments that are None, then we have **not** include them as keyword
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
# If we have arguments that are None, then we have **not** include them as keyword
# If we have arguments that are None, then we have to **not** include them as keyword

Copy link
Contributor

@sstadick sstadick left a comment

Choose a reason for hiding this comment

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

LGTM, this looks much nicer.

Copy link
Contributor

@sstadick sstadick left a comment

Choose a reason for hiding this comment

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

Continues to LGTM

@nh13 nh13 merged commit cd8a2b4 into master Jun 11, 2020
@nh13 nh13 deleted the bugfix/submit-job branch June 11, 2020 19:36
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