-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat: Add Support for DynamodbOnlineStoreConfig endpoint_url parameter #2485
Merged
feast-ci-bot
merged 8 commits into
feast-dev:master
from
TremaMiguel:feat/dynamodb-endpoint-url
Apr 8, 2022
Merged
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
af305e5
feat: support dynamodb client and resource with endpoint_url
TremaMiguel 2180e2c
fix: overwrite by partitionkey batchwrite operation
TremaMiguel 02f8071
docs: how to configure local dynamodb
TremaMiguel 76efcea
fix: DynamoDBonlineStore endpoint_url defaults to None
TremaMiguel 0440598
docs: setup dummy aws credentials
TremaMiguel 7d23380
test: DynamoDBOnlineStoreConfig endpoint_url configuration
TremaMiguel 4b9fcc6
Merge branch 'master' into feat/dynamodb-endpoint-url
TremaMiguel 77c9f13
feat: DynamoDBTable support endpoint_url
TremaMiguel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,7 @@ | |
from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple | ||
|
||
from pydantic import StrictStr | ||
from pydantic.typing import Literal | ||
from pydantic.typing import Literal, Union | ||
|
||
from feast import Entity, FeatureView, utils | ||
from feast.infra.infra_object import DYNAMODB_INFRA_OBJECT_CLASS_TYPE, InfraObject | ||
|
@@ -50,17 +50,20 @@ class DynamoDBOnlineStoreConfig(FeastConfigBaseModel): | |
type: Literal["dynamodb"] = "dynamodb" | ||
"""Online store type selector""" | ||
|
||
batch_size: int = 40 | ||
"""Number of items to retrieve in a DynamoDB BatchGetItem call.""" | ||
|
||
endpoint_url: Union[str, None] = None | ||
"""DynamoDB local development endpoint Url, i.e. http://localhost:8000""" | ||
|
||
region: StrictStr | ||
"""AWS Region Name""" | ||
|
||
table_name_template: StrictStr = "{project}.{table_name}" | ||
"""DynamoDB table name template""" | ||
|
||
sort_response: bool = True | ||
"""Whether or not to sort BatchGetItem response.""" | ||
|
||
batch_size: int = 40 | ||
"""Number of items to retrieve in a DynamoDB BatchGetItem call.""" | ||
table_name_template: StrictStr = "{project}.{table_name}" | ||
"""DynamoDB table name template""" | ||
|
||
|
||
class DynamoDBOnlineStore(OnlineStore): | ||
|
@@ -95,8 +98,12 @@ def update( | |
""" | ||
online_config = config.online_store | ||
assert isinstance(online_config, DynamoDBOnlineStoreConfig) | ||
dynamodb_client = self._get_dynamodb_client(online_config.region) | ||
dynamodb_resource = self._get_dynamodb_resource(online_config.region) | ||
dynamodb_client = self._get_dynamodb_client( | ||
online_config.region, online_config.endpoint_url | ||
) | ||
dynamodb_resource = self._get_dynamodb_resource( | ||
online_config.region, online_config.endpoint_url | ||
) | ||
|
||
for table_instance in tables_to_keep: | ||
try: | ||
|
@@ -141,7 +148,9 @@ def teardown( | |
""" | ||
online_config = config.online_store | ||
assert isinstance(online_config, DynamoDBOnlineStoreConfig) | ||
dynamodb_resource = self._get_dynamodb_resource(online_config.region) | ||
dynamodb_resource = self._get_dynamodb_resource( | ||
online_config.region, online_config.endpoint_url | ||
) | ||
|
||
for table in tables: | ||
_delete_table_idempotent( | ||
|
@@ -175,7 +184,9 @@ def online_write_batch( | |
""" | ||
online_config = config.online_store | ||
assert isinstance(online_config, DynamoDBOnlineStoreConfig) | ||
dynamodb_resource = self._get_dynamodb_resource(online_config.region) | ||
dynamodb_resource = self._get_dynamodb_resource( | ||
online_config.region, online_config.endpoint_url | ||
) | ||
|
||
table_instance = dynamodb_resource.Table( | ||
_get_table_name(online_config, config, table) | ||
|
@@ -217,7 +228,9 @@ def online_read( | |
""" | ||
online_config = config.online_store | ||
assert isinstance(online_config, DynamoDBOnlineStoreConfig) | ||
dynamodb_resource = self._get_dynamodb_resource(online_config.region) | ||
dynamodb_resource = self._get_dynamodb_resource( | ||
online_config.region, online_config.endpoint_url | ||
) | ||
table_instance = dynamodb_resource.Table( | ||
_get_table_name(online_config, config, table) | ||
) | ||
|
@@ -260,14 +273,16 @@ def online_read( | |
result.extend(batch_size_nones) | ||
return result | ||
|
||
def _get_dynamodb_client(self, region: str): | ||
def _get_dynamodb_client(self, region: str, endpoint_url: Optional[str] = None): | ||
if self._dynamodb_client is None: | ||
self._dynamodb_client = _initialize_dynamodb_client(region) | ||
self._dynamodb_client = _initialize_dynamodb_client(region, endpoint_url) | ||
return self._dynamodb_client | ||
|
||
def _get_dynamodb_resource(self, region: str): | ||
def _get_dynamodb_resource(self, region: str, endpoint_url: Optional[str] = None): | ||
if self._dynamodb_resource is None: | ||
self._dynamodb_resource = _initialize_dynamodb_resource(region) | ||
self._dynamodb_resource = _initialize_dynamodb_resource( | ||
region, endpoint_url | ||
) | ||
return self._dynamodb_resource | ||
|
||
def _sort_dynamodb_response(self, responses: list, order: list): | ||
|
@@ -285,12 +300,12 @@ def _sort_dynamodb_response(self, responses: list, order: list): | |
return table_responses_ordered | ||
|
||
|
||
def _initialize_dynamodb_client(region: str): | ||
return boto3.client("dynamodb", region_name=region) | ||
def _initialize_dynamodb_client(region: str, endpoint_url: Optional[str] = None): | ||
return boto3.client("dynamodb", region_name=region, endpoint_url=endpoint_url) | ||
|
||
|
||
def _initialize_dynamodb_resource(region: str): | ||
return boto3.resource("dynamodb", region_name=region) | ||
def _initialize_dynamodb_resource(region: str, endpoint_url: Optional[str] = None): | ||
return boto3.resource("dynamodb", region_name=region, endpoint_url=endpoint_url) | ||
|
||
|
||
# TODO(achals): This form of user-facing templating is experimental. | ||
|
@@ -362,8 +377,12 @@ def from_proto(dynamodb_table_proto: DynamoDBTableProto) -> Any: | |
) | ||
|
||
def update(self): | ||
dynamodb_client = _initialize_dynamodb_client(region=self.region) | ||
dynamodb_resource = _initialize_dynamodb_resource(region=self.region) | ||
dynamodb_client = _initialize_dynamodb_client( | ||
region=self.region, endpoint_url=None | ||
) | ||
dynamodb_resource = _initialize_dynamodb_resource( | ||
region=self.region, endpoint_url=None | ||
) | ||
|
||
try: | ||
dynamodb_resource.create_table( | ||
|
@@ -384,5 +403,7 @@ def update(self): | |
dynamodb_client.get_waiter("table_exists").wait(TableName=f"{self.name}") | ||
|
||
def teardown(self): | ||
dynamodb_resource = _initialize_dynamodb_resource(region=self.region) | ||
dynamodb_resource = _initialize_dynamodb_resource( | ||
region=self.region, endpoint_url=None | ||
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. same question 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. I've made some changes based on your suggestions, if you don't mind giving it a second check, please. |
||
) | ||
_delete_table_idempotent(dynamodb_resource, self.name) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the endpoint_url None here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@achals my mistake, should we add it as an attribute to
DynamoDBTable
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with how the user interacts with the
DynamoDBTable
class, and how can the user configure it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets add it for consistency, it's not currently used right now but will be used as part of upcoming features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok