diff --git a/.github/workflows/database-sweep.yaml b/.github/workflows/database-sweep.yaml index a975bc834..2ce0385da 100644 --- a/.github/workflows/database-sweep.yaml +++ b/.github/workflows/database-sweep.yaml @@ -8,10 +8,8 @@ name: Sweep test databases on: - workflow_run: - workflows: [Test] - branches: [master] - types: [completed] + schedule: + - cron: '0 3 * * *' workflow_dispatch: inputs: dry-run: @@ -28,15 +26,10 @@ concurrency: jobs: sweep: - # A separate completion workflow also runs after failed or cancelled tests. - if: >- - github.repository == 'pyathena-dev/PyAthena' && - ((github.event_name == 'workflow_run' && - github.event.workflow_run.event == 'schedule' && - github.event.workflow_run.head_repository.full_name == github.repository) || - (github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/master')) + # Scheduled runs use the default branch; manual dispatch must choose it. + if: github.repository == 'pyathena-dev/PyAthena' && github.ref == 'refs/heads/master' runs-on: ubuntu-latest - timeout-minutes: 15 + timeout-minutes: 60 permissions: contents: read id-token: write @@ -49,11 +42,8 @@ jobs: DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && inputs.dry-run && 'true' || 'false' }} steps: - # Never execute code or consume artifacts from the triggering test run. - - name: Checkout default branch - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: - ref: master persist-credentials: false - uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 diff --git a/docs/testing.md b/docs/testing.md index bf58fb77e..803ef28e0 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -95,7 +95,8 @@ AWS_ATHENA_S3_TABLES_CATALOG=s3tablescatalog/your-table-bucket Each test process creates its own namespace in the table bucket, named like its schema, and deletes it with any remaining tables at the end; with pytest-xdist that is each worker, not the controller. The test identity needs `s3tables:CreateNamespace`, `s3tables:DeleteNamespace`, `s3tables:ListTables`, and `s3tables:DeleteTable` on the table bucket. -A session that stops early leaves its namespace behind; `scripts/sweep_databases.py` removes such namespaces once they are more than seven days old. +A session that stops early leaves its namespace behind; `scripts/sweep_databases.py` removes such namespaces once they are more than one day old. +It also removes the database and namespace of a session still running after one day, if they are in the account, region, and table bucket that the sweep covers. The S3 Tables tests live in `tests/pyathena/sqlalchemy/test_base.py` and `tests/pyathena/test_glue.py` and run under `just test pyathena`, not the SQLAlchemy compliance-suite commands. Managed storage and S3 Tables tests skip when their respective optional configuration is absent. diff --git a/scripts/sweep_databases.py b/scripts/sweep_databases.py index c21821bb0..664076fc4 100644 --- a/scripts/sweep_databases.py +++ b/scripts/sweep_databases.py @@ -13,15 +13,18 @@ # AWS_PROFILE and AWS_DEFAULT_REGION can select the account and region. # # Eligible databases must exactly match a PyAthena or SQLAlchemy fixture name -# and have a creation time more than seven days old. Resource links and federated -# databases are excluded. The script completes inventory before deleting and -# rechecks eligibility and creation time immediately before each deletion. -# Only missing-database errors are ignored; other API failures stop the sweep. +# and have a creation time more than one day old, longer than a CI test session +# can run (a GitHub-hosted job stops after six hours). A local session against +# the same account that stays open longer can lose its database. Resource +# links and federated databases are excluded. The script completes inventory +# before deleting and rechecks eligibility and creation time immediately +# before each deletion. Only missing-database errors are ignored; other API +# failures stop the sweep. # Deletion removes Glue database and table metadata, not S3 objects. # # With AWS_ATHENA_S3_TABLES_CATALOG set (s3tablescatalog/), the # script also sweeps that table bucket's namespaces named like PyAthena test -# schemas and more than seven days old, deleting their tables first. Test +# schemas and more than one day old, deleting their tables first. Test # sessions create and delete such a namespace; a session that stops early # leaves it behind. The namespace ID is rechecked before its tables are deleted # and again before the namespace is deleted. Each table is deleted only if it @@ -29,11 +32,11 @@ # skipped. DeleteNamespace takes only a name, so an empty namespace recreated # under the same name right after the last recheck would still be deleted. # -# .github/workflows/database-sweep.yaml runs this script after scheduled Test -# runs complete on master, including failures and cancellations. It does not run -# after PR tests or manually dispatched tests. Manual sweep dispatch on master -# defaults to preview. The job has a 15-minute timeout; a timeout or API failure -# can leave eligible databases for a later run. +# .github/workflows/database-sweep.yaml runs this script daily on master, so +# while those runs succeed, a cancelled test run's leftovers are removed within +# about two days. Manual sweep dispatch on master defaults to preview. The job +# has a 60-minute timeout; a timeout or API failure can leave eligible databases +# for a later run. import argparse import contextlib @@ -54,6 +57,8 @@ rf"(?:{_PYATHENA_TEST_SCHEMA}|test_[0-9a-f]{{12}}(?:_test_schema(?:_2)?)?)" ) _TEST_NAMESPACE = re.compile(_PYATHENA_TEST_SCHEMA) +# Longer than a CI test session can run; see the module comment. +_RETENTION = timedelta(days=1) def _eligible(database: dict[str, Any], cutoff: datetime) -> bool: @@ -69,13 +74,21 @@ def _eligible(database: dict[str, Any], cutoff: datetime) -> bool: def sweep_databases(client: Any, catalog_id: str, *, dry_run: bool = True) -> dict[str, int]: - """Preview or delete test databases older than seven days. + """Preview or delete test databases older than one day. Fixtures generate fresh database names for each session or worker. - Databases younger than seven days are retained, including concurrent CI runs. + Databases younger than one day are retained, including concurrent CI runs. Only Glue metadata is deleted; S3 objects and child catalogs are untouched. + + Args: + client: A boto3 Glue client. + catalog_id: The ID of the Data Catalog to sweep. + dry_run: Only count eligible databases. + + Returns: + The numbers of eligible, deleted and skipped databases. """ - cutoff = datetime.now(UTC) - timedelta(days=7) + cutoff = datetime.now(UTC) - _RETENTION # Finish pagination before deleting anything from the catalog. candidates = [ database @@ -127,11 +140,11 @@ def _eligible_namespace(namespace: dict[str, Any], cutoff: datetime) -> bool: def sweep_s3tables_namespaces( client: Any, table_bucket_arn: str, *, dry_run: bool = True ) -> dict[str, int]: - """Preview or delete test S3 Tables namespaces older than seven days. + """Preview or delete test S3 Tables namespaces older than one day. Test sessions create a namespace named like their schema and delete it when they finish; this removes the ones a session left behind. Namespaces younger - than seven days are retained, including those of running sessions. + than one day are retained, including those of running sessions. Args: client: A boto3 S3 Tables client. @@ -141,7 +154,7 @@ def sweep_s3tables_namespaces( Returns: The numbers of eligible, deleted and skipped namespaces. """ - cutoff = datetime.now(UTC) - timedelta(days=7) + cutoff = datetime.now(UTC) - _RETENTION # Finish pagination before deleting anything from the table bucket. candidates = [ namespace diff --git a/scripts/tests/test_sweep_databases.py b/scripts/tests/test_sweep_databases.py index 29f9d611d..d46a8c638 100644 --- a/scripts/tests/test_sweep_databases.py +++ b/scripts/tests/test_sweep_databases.py @@ -105,6 +105,14 @@ def test_preview_and_apply_finish_pagination_before_mutating(glue): } +@pytest.mark.parametrize(("age", "eligible"), [(timedelta(days=2), 1), (timedelta(hours=12), 0)]) +def test_databases_expire_after_one_day(glue, age, eligible): + client, stubber = glue + database = {**DATABASE, "CreateTime": datetime.now(UTC) - age} + stubber.add_response("get_databases", {"DatabaseList": [database]}, {"CatalogId": CATALOG}) + assert sweep_databases(client, CATALOG)["eligible"] == eligible + + @pytest.mark.parametrize( "current", [ @@ -255,6 +263,18 @@ def test_only_expired_session_namespaces_are_eligible(properties, expected): assert _eligible_namespace({**NAMESPACE, **properties}, OLD + timedelta(days=1)) is expected +@pytest.mark.parametrize(("age", "eligible"), [(timedelta(days=2), 1), (timedelta(hours=12), 0)]) +def test_namespaces_expire_after_one_day(s3tables, age, eligible): + client, stubber = s3tables + namespace = {**NAMESPACE, "createdAt": datetime.now(UTC) - age} + stubber.add_response( + "list_namespaces", + {"namespaces": [namespace]}, + {"tableBucketARN": BUCKET_ARN, "prefix": "pyathena_test_"}, + ) + assert sweep_s3tables_namespaces(client, BUCKET_ARN)["eligible"] == eligible + + def test_namespace_sweep_deletes_tables_then_namespace(s3tables): client, stubber = s3tables listing = {"tableBucketARN": BUCKET_ARN, "prefix": "pyathena_test_"}