Skip to content

Commit

Permalink
Fix leaking dynamodb tables in integration tests (#2104)
Browse files Browse the repository at this point in the history
  • Loading branch information
pyalex authored Dec 10, 2021
1 parent 99f1851 commit 50f29cd
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 5 deletions.
1 change: 1 addition & 0 deletions .github/workflows/pr_integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,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}"
)
29 changes: 24 additions & 5 deletions sdk/python/tests/integration/registration/test_universal_types.py
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

0 comments on commit 50f29cd

Please sign in to comment.