-
Notifications
You must be signed in to change notification settings - Fork 429
feat(parameters): accept boto3_client to support private endpoints and ease testing #1096
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
Changes from 18 commits
ce851e9
b9b6753
159ca22
d33201e
2fbdca3
ac5554f
cd426fe
cd71ee0
ccc378b
2a77841
9b99f0b
d2c8f30
d854f1c
63bdde6
3528b8e
579f8e1
a8350ae
cec2f33
19ec9ae
58e2d5e
98dc539
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,18 @@ | |
""" | ||
|
||
|
||
from typing import Dict, Optional | ||
from typing import TYPE_CHECKING, Dict, Optional | ||
|
||
import boto3 | ||
from boto3.dynamodb.conditions import Key | ||
from botocore.config import Config | ||
|
||
from .base import BaseProvider | ||
|
||
if TYPE_CHECKING: | ||
from mypy_boto3_dynamodb import DynamoDBServiceResource | ||
from mypy_boto3_dynamodb.service_resource import Table | ||
|
||
|
||
class DynamoDBProvider(BaseProvider): | ||
""" | ||
|
@@ -31,7 +35,9 @@ class DynamoDBProvider(BaseProvider): | |
config: botocore.config.Config, optional | ||
Botocore configuration to pass during client initialization | ||
boto3_session : boto3.session.Session, optional | ||
Boto3 session to use for AWS API communication | ||
Boto3 session to create a boto3_client from | ||
boto3_client: DynamoDBServiceResource, optional | ||
Boto3 DynamoDB Resource Client to use; boto3_session will be ignored if both are provided | ||
|
||
Example | ||
------- | ||
|
@@ -152,15 +158,19 @@ def __init__( | |
endpoint_url: Optional[str] = None, | ||
config: Optional[Config] = None, | ||
boto3_session: Optional[boto3.session.Session] = None, | ||
boto3_client: Optional["DynamoDBServiceResource"] = None, | ||
This comment was marked as resolved.
Sorry, something went wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this kind of change docs with examples should also be added to avoid confusion between using client vs resource. note, the original request was for appconfig. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a customer without clarification this would be confusing and not everyone uses mypy types There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed on the docs, I will make those changes tomorrow (docs, not param name) and merge it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks clarity always helps. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. overall for With 8 parameters now, this is borderline code smell. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @heitorlessa the docs definitely helps! It might be a good general call out to recommend people use boto3 session where possible |
||
): | ||
""" | ||
Initialize the DynamoDB client | ||
""" | ||
|
||
config = config or Config() | ||
session = boto3_session or boto3.session.Session() | ||
|
||
self.table = session.resource("dynamodb", endpoint_url=endpoint_url, config=config).Table(table_name) | ||
self.client: "DynamoDBServiceResource" = self._build_boto3_resource_client( | ||
heitorlessa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
service_name="dynamodb", | ||
client=boto3_client, | ||
session=boto3_session, | ||
config=config, | ||
endpoint_url=endpoint_url, | ||
) | ||
self.table: "Table" = self.client.Table(table_name) | ||
|
||
self.key_attr = key_attr | ||
self.sort_attr = sort_attr | ||
|
@@ -183,7 +193,9 @@ def _get(self, name: str, **sdk_options) -> str: | |
# Explicit arguments will take precedence over keyword arguments | ||
sdk_options["Key"] = {self.key_attr: name} | ||
|
||
return self.table.get_item(**sdk_options)["Item"][self.value_attr] | ||
# maintenance: look for better ways to correctly type DynamoDB multiple return types | ||
# without a breaking change within ABC return type | ||
return self.table.get_item(**sdk_options)["Item"][self.value_attr] # type: ignore[return-value] | ||
|
||
def _get_multiple(self, path: str, **sdk_options) -> Dict[str, str]: | ||
""" | ||
|
@@ -209,4 +221,6 @@ def _get_multiple(self, path: str, **sdk_options) -> Dict[str, str]: | |
response = self.table.query(**sdk_options) | ||
items.extend(response.get("Items", [])) | ||
|
||
return {item[self.sort_attr]: item[self.value_attr] for item in items} | ||
# maintenance: look for better ways to correctly type DynamoDB multiple return types | ||
# without a breaking change within ABC return type | ||
return {item[self.sort_attr]: item[self.value_attr] for item in items} # type: ignore[misc] |
Uh oh!
There was an error while loading. Please reload this page.