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

Fix leaking dynamodb tables in integration tests #2104

Merged
merged 6 commits into from
Dec 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/pr_integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ jobs:
- name: Install dependencies
run: make install-python-ci-dependencies
- name: Test python
if: ${{ always() }} # this will guarantee that step won't be canceled and resources won't leak
env:
FEAST_SERVER_DOCKER_IMAGE_TAG: ${{ needs.build-docker-image.outputs.DOCKER_IMAGE_TAG }}
FEAST_USAGE: "False"
Expand Down
12 changes: 12 additions & 0 deletions sdk/python/feast/infra/online_stores/dynamodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from datetime import datetime
from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple, Union

Expand All @@ -34,6 +35,9 @@
raise FeastExtrasDependencyImportError("aws", str(e))


logger = logging.getLogger(__name__)


class DynamoDBOnlineStoreConfig(FeastConfigBaseModel):
"""Online store config for DynamoDB store"""

Expand Down Expand Up @@ -179,9 +183,17 @@ def _delete_tables_idempotent(
f"{config.project}.{table_instance.name}"
)
table.delete()
logger.info(
f"Dynamo table {config.project}.{table_instance.name} was deleted"
)
except ClientError as ce:
# If the table deletion fails with ResourceNotFoundException,
# it means the table has already been deleted.
# Otherwise, re-raise the exception
if ce.response["Error"]["Code"] != "ResourceNotFoundException":
raise
else:
logger.warning(
f"Trying to delete table that doesn't exist:"
f" {config.project}.{table_instance.name}"
)
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
from dataclasses import dataclass
from datetime import datetime, timedelta
from typing import List
Expand All @@ -18,6 +19,8 @@
from tests.integration.feature_repos.universal.entities import driver
from tests.integration.feature_repos.universal.feature_views import driver_feature_view

logger = logging.getLogger(__name__)


def populate_test_configs(offline: bool):
entity_type_feature_dtypes = [
Expand Down Expand Up @@ -106,14 +109,19 @@ def get_fixtures(request):
field_mapping={"ts_1": "ts"},
)
fv = create_feature_view(
request.fixturename,
config.feature_dtype,
config.feature_is_list,
config.has_empty_list,
data_source,
)

def cleanup():
type_test_environment.data_source_creator.teardown()
try:
type_test_environment.data_source_creator.teardown()
except Exception: # noqa
logger.exception("DataSourceCreator teardown has failed")

type_test_environment.feature_store.teardown()

request.addfinalizer(cleanup)
Expand Down Expand Up @@ -151,7 +159,11 @@ def test_feature_get_historical_features_types_match(offline_types_test_fixtures
environment, config, data_source, fv = offline_types_test_fixtures
fs = environment.feature_store
fv = create_feature_view(
config.feature_dtype, config.feature_is_list, config.has_empty_list, data_source
"get_historical_features_types_match",
config.feature_dtype,
config.feature_is_list,
config.has_empty_list,
data_source,
)
entity = driver()
fs.apply([fv, entity])
Expand Down Expand Up @@ -197,7 +209,11 @@ def test_feature_get_historical_features_types_match(offline_types_test_fixtures
def test_feature_get_online_features_types_match(online_types_test_fixtures):
environment, config, data_source, fv = online_types_test_fixtures
fv = create_feature_view(
config.feature_dtype, config.feature_is_list, config.has_empty_list, data_source
"get_online_features_types_match",
config.feature_dtype,
config.feature_is_list,
config.has_empty_list,
data_source,
)
fs = environment.feature_store
features = [fv.name + ":value"]
Expand Down Expand Up @@ -230,7 +246,9 @@ def test_feature_get_online_features_types_match(online_types_test_fixtures):
assert isinstance(feature, expected_dtype)


def create_feature_view(feature_dtype, feature_is_list, has_empty_list, data_source):
def create_feature_view(
name, feature_dtype, feature_is_list, has_empty_list, data_source
):
if feature_is_list is True:
if feature_dtype == "int32":
value_type = ValueType.INT32_LIST
Expand All @@ -249,7 +267,8 @@ def create_feature_view(feature_dtype, feature_is_list, has_empty_list, data_sou
value_type = ValueType.FLOAT
elif feature_dtype == "bool":
value_type = ValueType.BOOL
return driver_feature_view(data_source, value_type=value_type,)

return driver_feature_view(data_source, name=name, value_type=value_type,)


def assert_expected_historical_feature_types(
Expand Down