Added `databricks labs ucx revert-migrated-table` command #729

FastLee · 2023-12-22T15:33:03Z

closes #671
revert-migrated-table scans HMS for tables marked as upgraded.
Looking for the upgraded_to property.
Clears the upgraded_to property to revert the table and allow it to be migrated again.
--schema and --table params are optional and allow scoping the command down to a schema or a single table.

codecov · 2023-12-22T15:34:28Z

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (6c89e67) 79.43% compared to head (365de62) 79.45%.

Files	Patch %	Lines
src/databricks/labs/ucx/cli.py	11.11%	16 Missing ⚠️
src/databricks/labs/ucx/hive_metastore/tables.py	94.25%	1 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #729      +/-   ##
==========================================
+ Coverage   79.43%   79.45%   +0.01%     
==========================================
  Files          42       42              
  Lines        4411     4516     +105     
  Branches      819      845      +26     
==========================================
+ Hits         3504     3588      +84     
- Misses        693      710      +17     
- Partials      214      218       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nfx

missing integration tests

src/databricks/labs/ucx/hive_metastore/tables.py

tests/unit/test_cli.py

nfx

Code is incorrect and has some bugs

src/databricks/labs/ucx/cli.py

src/databricks/labs/ucx/hive_metastore/tables.py

tests/integration/hive_metastore/test_migrate.py

tests/unit/hive_metastore/test_migrate.py

nfx · 2023-12-23T18:45:49Z

@FastLee dont we have to remove the table from UC as well?

FastLee · 2023-12-24T01:40:31Z

@FastLee dont we have to remove the table from UC as well?

I wouldn't mess with removing the table (for now)

nfx · 2023-12-24T11:10:59Z

I wouldn't mess with removing the table (for now)

@FastLee but then we'd have a corrupt state - HMS thinks table is not migrated, but UC has it migrated already. Incorrect behavior.

Perhaps tune the logic the following way:

If table is managed, prevent any reverts
if table is external, prompt confirmation before we remove the table and update table properties

nfx

Comments were not addressed

src/databricks/labs/ucx/hive_metastore/tables.py

tests/integration/hive_metastore/test_migrate.py

nfx

Still has incorrect code

labs.yml

src/databricks/labs/ucx/cli.py

src/databricks/labs/ucx/hive_metastore/tables.py

src/databricks/labs/ucx/cli.py

src/databricks/labs/ucx/hive_metastore/tables.py

src/databricks/labs/ucx/cli.py

nfx

Fix codecov checks to pass
invoke real migration methods in the integration tests, not fake
see other minor comments, that still have to be addressed

It might be ready for merge after that

labs.yml

src/databricks/labs/ucx/cli.py

src/databricks/labs/ucx/hive_metastore/tables.py

tests/integration/conftest.py

tests/integration/hive_metastore/test_migrate.py

…ty rather than the "upgraded_to" logic.

nfx

Looks good. Add more unit tests to fix codecov failing checks

src/databricks/labs/ucx/hive_metastore/tables.py

tests/integration/conftest.py

src/databricks/labs/ucx/hive_metastore/tables.py

nfx

Some unit tests are incorrect because they change private state instead of mocking depending component interactions

tests/unit/hive_metastore/test_migrate.py

* Added `databricks labs ucx repair-run --step ...` CLI command for repair run of any failed workflows, like `assessment`, `migrate-groups` etc. ([#724](#724)). * Added `databricks labs ucx revert-migrated-table` command ([#729](#729)). * Allow specifying a group list when group match options are used ([#725](#725)). * Fixed installation issue when upgrading from an older version of the tool and improve logs ([#740](#740)). * Renamed summary panel from Failure Summary to Assessment Summary ([#733](#733)). * Retry internal error when getting permissions and update legacy table ACL documentation ([#728](#728)). * Speedup installer execution ([#727](#727)).

FastLee had a problem deploying to account-admin December 22, 2023 15:33 — with GitHub Actions Failure

FastLee force-pushed the feature/revert_migrated_table_671 branch from 9c8f5b2 to 1c9dd4e Compare December 22, 2023 18:31

FastLee had a problem deploying to account-admin December 22, 2023 18:31 — with GitHub Actions Failure

FastLee had a problem deploying to account-admin December 22, 2023 19:01 — with GitHub Actions Failure

FastLee marked this pull request as ready for review December 22, 2023 19:01

FastLee requested review from a team and HariGS-DB December 22, 2023 19:01

FastLee force-pushed the feature/revert_migrated_table_671 branch from 8c1bdef to f353a41 Compare December 22, 2023 19:39

FastLee temporarily deployed to account-admin December 22, 2023 19:39 — with GitHub Actions Inactive

nfx requested changes Dec 22, 2023

View reviewed changes

FastLee force-pushed the feature/revert_migrated_table_671 branch from f353a41 to d63d11f Compare December 23, 2023 15:35

FastLee temporarily deployed to account-admin December 23, 2023 15:35 — with GitHub Actions Inactive

FastLee requested a review from nfx December 23, 2023 15:38

nfx requested changes Dec 23, 2023

View reviewed changes

FastLee temporarily deployed to account-admin December 24, 2023 03:18 — with GitHub Actions Inactive

FastLee force-pushed the feature/revert_migrated_table_671 branch from 4fc4103 to da73f7f Compare December 24, 2023 03:18

FastLee had a problem deploying to account-admin December 24, 2023 03:18 — with GitHub Actions Failure

FastLee requested a review from nfx December 24, 2023 03:19

nfx requested changes Dec 24, 2023

View reviewed changes

FastLee temporarily deployed to account-admin December 25, 2023 04:21 — with GitHub Actions Inactive

FastLee requested a review from nfx December 25, 2023 04:23

nfx requested changes Dec 25, 2023

View reviewed changes

nfx reviewed Dec 25, 2023

View reviewed changes

src/databricks/labs/ucx/cli.py Outdated Show resolved Hide resolved

FastLee had a problem deploying to account-admin December 25, 2023 17:33 — with GitHub Actions Failure

FastLee had a problem deploying to account-admin December 25, 2023 17:34 — with GitHub Actions Failure

FastLee requested a review from nfx December 25, 2023 17:35

FastLee added 2 commits December 26, 2023 08:26

Added support for views. Addressed comments.

5674f44

Added support for multithreading.

6c930e2

FastLee force-pushed the feature/revert_migrated_table_671 branch from 95d2f5e to 6c930e2 Compare December 26, 2023 13:26

FastLee temporarily deployed to account-admin December 26, 2023 13:26 — with GitHub Actions Inactive

nfx requested changes Dec 27, 2023

View reviewed changes

Addressed comments. Changed the logic to rely on upgraded_from proper…

426cdfd

…ty rather than the "upgraded_to" logic.

FastLee temporarily deployed to account-admin December 27, 2023 23:22 — with GitHub Actions Inactive

FastLee requested a review from nfx December 27, 2023 23:22

FastLee temporarily deployed to account-admin December 27, 2023 23:47 — with GitHub Actions Inactive

nfx approved these changes Dec 27, 2023

View reviewed changes

src/databricks/labs/ucx/hive_metastore/tables.py Show resolved Hide resolved

tests/integration/conftest.py Outdated Show resolved Hide resolved

src/databricks/labs/ucx/hive_metastore/tables.py Outdated Show resolved Hide resolved

Added tests to increase coverage.

c1446a3

FastLee force-pushed the feature/revert_migrated_table_671 branch from 58aa343 to c1446a3 Compare December 28, 2023 00:03

FastLee temporarily deployed to account-admin December 28, 2023 00:03 — with GitHub Actions Inactive

Addressed Comments.

55a1226

FastLee had a problem deploying to account-admin December 28, 2023 00:10 — with GitHub Actions Failure

FastLee requested a review from nfx December 28, 2023 00:11

nfx requested changes Dec 28, 2023

View reviewed changes

tests/unit/hive_metastore/test_migrate.py Outdated Show resolved Hide resolved

tests/unit/hive_metastore/test_migrate.py Outdated Show resolved Hide resolved

tests/unit/hive_metastore/test_migrate.py Show resolved Hide resolved

Improved unit testing.

365de62

FastLee temporarily deployed to account-admin December 28, 2023 02:42 — with GitHub Actions Inactive

FastLee requested a review from nfx December 28, 2023 02:43

nfx approved these changes Dec 28, 2023

View reviewed changes

nfx changed the title ~~CLI Command databricks labs ucx revert-migrated-table~~ Added databricks labs ucx revert-migrated-table command Dec 28, 2023

nfx merged commit 473f137 into main Dec 28, 2023
7 checks passed

nfx deleted the feature/revert_migrated_table_671 branch December 28, 2023 09:02

nfx mentioned this pull request Dec 28, 2023

Release v0.8.0 #744

Merged

FastLee added a commit that referenced this pull request Jan 19, 2024

Added databricks labs ucx revert-migrated-table command (#729)

2a78b0e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added `databricks labs ucx revert-migrated-table` command #729

Added `databricks labs ucx revert-migrated-table` command #729

FastLee commented Dec 22, 2023 •

edited

Loading

codecov bot commented Dec 22, 2023 •

edited

Loading

nfx left a comment

nfx left a comment

nfx commented Dec 23, 2023

FastLee commented Dec 24, 2023

nfx commented Dec 24, 2023

nfx left a comment

nfx left a comment

nfx left a comment

nfx left a comment

nfx left a comment

Added databricks labs ucx revert-migrated-table command #729

Added databricks labs ucx revert-migrated-table command #729

Conversation

FastLee commented Dec 22, 2023 • edited Loading

codecov bot commented Dec 22, 2023 • edited Loading

Codecov Report

nfx left a comment

Choose a reason for hiding this comment

nfx left a comment

Choose a reason for hiding this comment

nfx commented Dec 23, 2023

FastLee commented Dec 24, 2023

nfx commented Dec 24, 2023

nfx left a comment

Choose a reason for hiding this comment

nfx left a comment

Choose a reason for hiding this comment

nfx left a comment

Choose a reason for hiding this comment

nfx left a comment

Choose a reason for hiding this comment

nfx left a comment

Choose a reason for hiding this comment

Added `databricks labs ucx revert-migrated-table` command #729

Added `databricks labs ucx revert-migrated-table` command #729

FastLee commented Dec 22, 2023 •

edited

Loading

codecov bot commented Dec 22, 2023 •

edited

Loading