astro-sdk icon indicating copy to clipboard operation
astro-sdk copied to clipboard

Fix cleanup all tables

Open feluelle opened this issue 3 years ago • 1 comments

Description

What is the current behavior?

Currently, aql.cleanup() does not cleanup/remove all temp tables.

We know this for sure for Snowflake:

The following CI logs were giving us the hint that the tables are not being deleted after running the tests.

E       sqlalchemy.exc.ProgrammingError: (snowflake.connector.errors.ProgrammingError) 090153 (22000): 01a7981b-0605-9c81-0000-68211381e012: The result set size exceeded the max number of rows(10000) supported for SHOW statements. Use LIMIT option to limit result set to a smaller number.
E       [SQL: SHOW /* sqlalchemy:get_table_names */ TABLES IN astroflow_ci]
E       (Background on this error at: https://sqlalche.me/e/14/f405)

The schema where we write the tables to exceeded 10000 tables.

(probably) related: #963

What is the new behavior?

Fix cleanup all tables

  • run cleanup tests on all databases

Does this introduce a breaking change?

No, but we will remove temp tables when the issue is fixed.

Checklist

  • [x] Created tests which fail without the change (if possible)
  • [ ] Extended the README / documentation, if necessary

feluelle avatar Oct 13 '22 10:10 feluelle

Codecov Report

Base: 93.82% // Head: 94.17% // Increases project coverage by +0.35% :tada:

Coverage data is based on head (15a3903) compared to base (0173a07). Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1053      +/-   ##
==========================================
+ Coverage   93.82%   94.17%   +0.35%     
==========================================
  Files          65       13      -52     
  Lines        3044      498    -2546     
  Branches      341       50     -291     
==========================================
- Hits         2856      469    -2387     
+ Misses        129       20     -109     
+ Partials       59        9      -50     
Impacted Files Coverage Δ
sql-cli/sql_cli/__main__.py 100.00% <0.00%> (ø)
python-sdk/src/astro/databases/base.py
python-sdk/src/astro/sql/operators/cleanup.py
...sdk/src/astro/sql/operators/upstream_task_mixin.py
python-sdk/src/astro/files/locations/google/gcs.py
python-sdk/src/astro/files/locations/local.py
python-sdk/src/astro/databases/__init__.py
python-sdk/src/astro/databases/snowflake.py
python-sdk/src/astro/files/locations/base.py
python-sdk/src/astro/sql/operators/append.py
... and 44 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Oct 13 '22 10:10 codecov[bot]

FYI There are some weird issues going on with the test_cleanup_non_temp_table test. Running them one by one works as expected but running all together fails for some. I will continue to investigate..

feluelle avatar Oct 31 '22 15:10 feluelle

Fixed the issues related to cleanup. All current failing issues are unrelated. They are also failing in main.

feluelle avatar Nov 01 '22 13:11 feluelle