boto / boto3

AWS SDK for Python
https://aws.amazon.com/sdk-for-python/
Apache License 2.0
8.93k stars 1.86k forks source link

Allow passing a sentinel NONE (instead of None) for kwargs that get omitted #2813

Open tuukkamustonen opened 3 years ago

tuukkamustonen commented 3 years ago

Currently, you cannot do:

config = {}  # loaded from somewhere
ec2.create_volume(
    ...,
    KmsKeyId=config.get('kms_key_id')
)

...with the idea that if your config doesn't specify kms_key_id property (or if it's None) then KmsKeyId argument would make no impact. Instead, you get:

botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter KmsKeyId, value: None, type: <type 'NoneType'>, valid types: <type 'basestring'>

Working around it leads into hacks like:

opts = {...}
if config.get('kms_key_id'):
    opts['KmsKeyId'] = config.get('kms_key_id')
ec2.create_volume(
    ...,
    **opts
)

But that loses you type hinting support (if you're using something like https://pypi.org/project/boto3-stubs). And mypy will whine you about it.

It's understandable why implementation rejects None values like this (see https://github.com/boto/boto3/issues/400#issuecomment-622501123 as an explanation). Not proposing to change that.

However, it would be nicer if we could pass in a sentinel value, as a value that should simply have no impact:

from boto3 import NONE

ec2.create_volume(
    ...,
    KmsKeyId=config.get('kms_key_id', NONE)
)

This would be the same as not passing in KmsKeyId at all.

See https://github.com/boto/boto3/issues/331 and https://github.com/boto/boto3/issues/400 as closed old issues about the same topic (but without suggestion to allow sentinel value).

stobrien89 commented 3 years ago

Hi @tuukkamustonen,

Thanks for the feature request! I can definitely see where this would be useful. I'll pass along to the team for review, although I can't make any guarantees as to when/if this will be implemented.

leoskyrocker commented 2 years ago

Hi, just here to upvote and provide a use case. Currently, it is hard to write while loops for paginated API calls without verbosely expanding the call twice:

def foo():
        response = self.client.get_query_results(
            EventDataStore=event_store_id,
            QueryId=query_id,
            MaxQueryResults=10,
        )
        next_token = response.get("NextToken", None)
        yield response["QueryResultRows"]

        while next_token:
            response = self.client.get_query_results(
                EventDataStore=event_store_id,
                QueryId=query_id,
                MaxQueryResults=10,
                NextToken=next_token,
            )
            next_token = response.get("NextToken", None)
            yield response["QueryResultRows"]

In here, NextToken is only needed if it is present from the last response. If we called initially with the next token, boto3 would complain that it cannot be of type None. If this validation is improved, one can simply reduce the above code down to:

def foo():
        next_token = None
        while next_token:
            response = self.client.get_query_results(
                EventDataStore=event_store_id,
                QueryId=query_id,
                MaxQueryResults=10,
                NextToken=next_token,
            )
            next_token = response.get("NextToken", None)
            yield response["QueryResultRows"]

This is a slightly better version with the current limitation, but it is still not great:

def foo():
        is_first_call = True
        while next_token or is_first_call:
            request_body = {
                "EventDataStore": event_store_id,
                "QueryId": query_id,
                "MaxQueryResults": 10,
                "NextToken": next_token,
            }
            if not next_token:
                del request_body["NextToken"]
            response = self.client.get_query_results(**request_body)

            is_first_call, next_token = False, response.get("NextToken", None)
            yield response["QueryResultRows"]

(Not so relevant - the client here is the cloudtrail sdk client)

BaxHugh commented 1 month ago

Here to upvote as well, this is a very much needed feature please!