Skip to content

Commit

Permalink
Use dedicated advice code for uncomputed values (#2019)
Browse files Browse the repository at this point in the history
## Changes
use dedicated advice code for uncomputed values

### Linked issues
None

### Functionality 
None

### Tests
- [x] ran unit tests

Co-authored-by: Eric Vergnaud <[email protected]>
  • Loading branch information
ericvergnaud and ericvergnaud authored Jul 2, 2024
1 parent e3a5e31 commit b11ac05
Show file tree
Hide file tree
Showing 21 changed files with 40 additions and 40 deletions.
2 changes: 1 addition & 1 deletion src/databricks/labs/ucx/source_code/linters/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def _raise_advice_if_unresolved(cls, node: NodeNG, session_state: CurrentSession
if has_unresolved:
yield from [
Advisory.from_node(
'dbutils-notebook-run-dynamic',
'notebook-run-cannot-compute-value',
"Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)",
node=node,
)
Expand Down
6 changes: 3 additions & 3 deletions src/databricks/labs/ucx/source_code/linters/pyspark.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def lint(
yield from self._lint_table_arg(from_table, node, inferred)
except InferenceError:
yield Advisory.from_node(
code='table-migrate',
code='table-migrate-cannot-compute-value',
message=f"Can't migrate table_name argument in '{node.as_string()}' because its value cannot be computed",
node=node,
)
Expand All @@ -94,7 +94,7 @@ def _lint_table_arg(cls, from_table: FromTable, call_node: NodeNG, inferred: Inf
yield advice.replace_from_node(call_node)
else:
yield Advisory.from_node(
code='table-migrate',
code='table-migrate-cannot-compute-value',
message=f"Can't migrate table_name argument in '{call_node.as_string()}' because its value cannot be computed",
node=call_node,
)
Expand All @@ -117,7 +117,7 @@ def lint(
for inferred in Tree(table_arg).infer_values(session_state):
if not inferred.is_inferred():
yield Advisory.from_node(
code='table-migrate',
code='table-migrate-cannot-compute-value',
message=f"Can't migrate '{node.as_string()}' because its table name argument cannot be computed",
node=node,
)
Expand Down
4 changes: 2 additions & 2 deletions src/databricks/labs/ucx/source_code/notebooks/cells.py
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ def _register_notebook(self, base_node: NotebookRunCall):
has_unresolved, paths = base_node.get_notebook_paths(self._context.session_state)
if has_unresolved:
yield DependencyProblem(
'dependency-cannot-compute',
'dependency-cannot-compute-value',
f"Can't check dependency from {base_node.node.as_string()} because the expression cannot be computed",
)
for path in paths:
Expand All @@ -451,7 +451,7 @@ def _register_notebook(self, base_node: NotebookRunCall):
def _mutate_path_lookup(self, change: SysPathChange):
if isinstance(change, UnresolvedPath):
yield DependencyProblem(
'sys-path-cannot-compute',
'sys-path-cannot-compute-value',
f"Can't update sys.path from {change.node.as_string()} because the expression cannot be computed",
)
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
print(len(result))
index = 10
spark.sql(f"SELECT * FROM table_{index}").collect()
# ucx[table-migrate:+2:0:+2:40] Can't migrate table_name argument in 'spark.sql(f'SELECT * FROM {table_name}')' because its value cannot be computed
# ucx[table-migrate-cannot-compute-value:+2:0:+2:40] Can't migrate table_name argument in 'spark.sql(f'SELECT * FROM {table_name}')' because its value cannot be computed
table_name = f"table_{index}"
spark.sql(f"SELECT * FROM {table_name}").collect()
# ucx[table-migrate:+4:4:+4:20] Table old.things is migrated to brand.new.stuff in Unity Catalog
# ucx[table-migrate:+3:4:+3:20] Can't migrate table_name argument in 'spark.sql(query)' because its value cannot be computed
# ucx[table-migrate-cannot-compute-value:+3:4:+3:20] Can't migrate table_name argument in 'spark.sql(query)' because its value cannot be computed
table_name = f"table_{index}"
for query in ["SELECT * FROM old.things", f"SELECT * FROM {table_name}"]:
spark.sql(query).collect()
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
a = "./leaf1.py"
dbutils.notebook.run(a)
b = some_function()
# ucx[dbutils-notebook-run-dynamic:+1:0:+1:23] Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)
# ucx[notebook-run-cannot-compute-value:+1:0:+1:23] Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)
dbutils.notebook.run(b)
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
spark.catalog.cacheTable(storageLevel=None, tableName="old.things")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:0:+1:30] Can't migrate 'spark.catalog.cacheTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:0:+1:30] Can't migrate 'spark.catalog.cacheTable(name)' because its table name argument cannot be computed
spark.catalog.cacheTable(name)
# ucx[table-migrate:+1:0:+1:40] Can't migrate 'spark.catalog.cacheTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:0:+1:40] Can't migrate 'spark.catalog.cacheTable(f'boop{stuff}')' because its table name argument cannot be computed
spark.catalog.cacheTable(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
do_stuff_with(df)

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:9:+1:48] Can't migrate 'spark.catalog.createExternalTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:9:+1:48] Can't migrate 'spark.catalog.createExternalTable(name)' because its table name argument cannot be computed
df = spark.catalog.createExternalTable(name)
do_stuff_with(df)
# ucx[table-migrate:+1:9:+1:58] Can't migrate 'spark.catalog.createExternalTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:9:+1:58] Can't migrate 'spark.catalog.createExternalTable(f'boop{stuff}')' because its table name argument cannot be computed
df = spark.catalog.createExternalTable(f"boop{stuff}")
do_stuff_with(df)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
do_stuff_with(df)

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:9:+1:40] Can't migrate 'spark.catalog.createTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:9:+1:40] Can't migrate 'spark.catalog.createTable(name)' because its table name argument cannot be computed
df = spark.catalog.createTable(name)
do_stuff_with(df)
# ucx[table-migrate:+1:9:+1:50] Can't migrate 'spark.catalog.createTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:9:+1:50] Can't migrate 'spark.catalog.createTable(f'boop{stuff}')' because its table name argument cannot be computed
df = spark.catalog.createTable(f"boop{stuff}")
do_stuff_with(df)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@
do_stuff_with(table)

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:12:+1:40] Can't migrate 'spark.catalog.getTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:12:+1:40] Can't migrate 'spark.catalog.getTable(name)' because its table name argument cannot be computed
table = spark.catalog.getTable(name)
do_stuff_with(table)
# ucx[table-migrate:+1:12:+1:50] Can't migrate 'spark.catalog.getTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:12:+1:50] Can't migrate 'spark.catalog.getTable(f'boop{stuff}')' because its table name argument cannot be computed
table = spark.catalog.getTable(f"boop{stuff}")
do_stuff_with(table)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
cached_previously = spark.catalog.isCached("old.things", "extra-argument")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:24:+1:52] Can't migrate 'spark.catalog.isCached(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:24:+1:52] Can't migrate 'spark.catalog.isCached(name)' because its table name argument cannot be computed
cached_previously = spark.catalog.isCached(name)
# ucx[table-migrate:+1:24:+1:62] Can't migrate 'spark.catalog.isCached(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:24:+1:62] Can't migrate 'spark.catalog.isCached(f'boop{stuff}')' because its table name argument cannot be computed
cached_previously = spark.catalog.isCached(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
columns = spark.catalog.listColumns(dbName="old", name="things")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:14:+1:45] Can't migrate 'spark.catalog.listColumns(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:14:+1:45] Can't migrate 'spark.catalog.listColumns(name)' because its table name argument cannot be computed
columns = spark.catalog.listColumns(name)
# ucx[table-migrate:+1:14:+1:55] Can't migrate 'spark.catalog.listColumns(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:14:+1:55] Can't migrate 'spark.catalog.listColumns(f'boop{stuff}')' because its table name argument cannot be computed
columns = spark.catalog.listColumns(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
spark.catalog.recoverPartitions("old.things", "extra-argument")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:4:+1:41] Can't migrate 'spark.catalog.recoverPartitions(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:41] Can't migrate 'spark.catalog.recoverPartitions(name)' because its table name argument cannot be computed
spark.catalog.recoverPartitions(name)
# ucx[table-migrate:+1:4:+1:51] Can't migrate 'spark.catalog.recoverPartitions(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:51] Can't migrate 'spark.catalog.recoverPartitions(f'boop{stuff}')' because its table name argument cannot be computed
spark.catalog.recoverPartitions(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
spark.catalog.refreshTable("old.things", "extra-argument")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:4:+1:36] Can't migrate 'spark.catalog.refreshTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:36] Can't migrate 'spark.catalog.refreshTable(name)' because its table name argument cannot be computed
spark.catalog.refreshTable(name)
# ucx[table-migrate:+1:4:+1:46] Can't migrate 'spark.catalog.refreshTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:46] Can't migrate 'spark.catalog.refreshTable(f'boop{stuff}')' because its table name argument cannot be computed
spark.catalog.refreshTable(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@
pass

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:7:+1:38] Can't migrate 'spark.catalog.tableExists(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:7:+1:38] Can't migrate 'spark.catalog.tableExists(name)' because its table name argument cannot be computed
if spark.catalog.tableExists(name):
pass
# ucx[table-migrate:+1:7:+1:48] Can't migrate 'spark.catalog.tableExists(f'boot{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:7:+1:48] Can't migrate 'spark.catalog.tableExists(f'boot{stuff}')' because its table name argument cannot be computed
if spark.catalog.tableExists(f"boot{stuff}"):
pass

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
spark.catalog.uncacheTable("old.things", "extra-argument")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:4:+1:36] Can't migrate 'spark.catalog.uncacheTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:36] Can't migrate 'spark.catalog.uncacheTable(name)' because its table name argument cannot be computed
spark.catalog.uncacheTable(name)
# ucx[table-migrate:+1:4:+1:46] Can't migrate 'spark.catalog.uncacheTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:46] Can't migrate 'spark.catalog.uncacheTable(f'boop{stuff}')' because its table name argument cannot be computed
spark.catalog.uncacheTable(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
df.write.insertInto(overwrite=None, tableName="old.things")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:4:+1:29] Can't migrate 'df.write.insertInto(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:29] Can't migrate 'df.write.insertInto(name)' because its table name argument cannot be computed
df.write.insertInto(name)
# ucx[table-migrate:+1:4:+1:39] Can't migrate 'df.write.insertInto(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:39] Can't migrate 'df.write.insertInto(f'boop{stuff}')' because its table name argument cannot be computed
df.write.insertInto(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
df.write.saveAsTable(format="xyz", name="old.things")

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+1:4:+1:46] Can't migrate 'df.write.format('delta').saveAsTable(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:46] Can't migrate 'df.write.format('delta').saveAsTable(name)' because its table name argument cannot be computed
df.write.format("delta").saveAsTable(name)
# ucx[table-migrate:+1:4:+1:56] Can't migrate 'df.write.format('delta').saveAsTable(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+1:4:+1:56] Can't migrate 'df.write.format('delta').saveAsTable(f'boop{stuff}')' because its table name argument cannot be computed
df.write.format("delta").saveAsTable(f"boop{stuff}")

## Some trivial references to the method or table in unrelated contexts that should not trigger warnigns.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
do_stuff_with(df)

## Some calls that use a variable whose value is unknown: they could potentially reference a migrated table.
# ucx[table-migrate:+3:9:+3:26] Can't migrate 'spark.table(name)' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+3:9:+3:26] Can't migrate 'spark.table(name)' because its table name argument cannot be computed
# TODO: Fix false positive:
# ucx[table-migrate:+1:9:+1:26] The default format changed in Databricks Runtime 8.0, from Parquet to Delta
df = spark.table(name)
do_stuff_with(df)
# ucx[table-migrate:+3:9:+3:36] Can't migrate 'spark.table(f'boop{stuff}')' because its table name argument cannot be computed
# ucx[table-migrate-cannot-compute-value:+3:9:+3:36] Can't migrate 'spark.table(f'boop{stuff}')' because its table name argument cannot be computed
# TODO: Fix false positive:
# ucx[table-migrate:+1:9:+1:36] The default format changed in Databricks Runtime 8.0, from Parquet to Delta
df = spark.table(f"boop{stuff}")
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/source_code/samples/functional/widgets.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
path = dbutils.widgets.get("my-widget")
dbutils.notebook.run(path)
path = dbutils.widgets.get("no-widget")
# ucx[dbutils-notebook-run-dynamic:+1:0:+1:26] Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)
# ucx[notebook-run-cannot-compute-value:+1:0:+1:26] Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)
dbutils.notebook.run(path)
values = dbutils.widgets.getAll()
path = values["my-widget"]
dbutils.notebook.run(path)
path = values["no-widget"]
# ucx[dbutils-notebook-run-dynamic:+1:0:+1:26] Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)
# ucx[notebook-run-cannot-compute-value:+1:0:+1:26] Path for 'dbutils.notebook.run' cannot be computed and requires adjusting the notebook path(s)
dbutils.notebook.run(path)
4 changes: 2 additions & 2 deletions tests/unit/source_code/test_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def test_dependency_resolver_raises_problem_for_non_inferable_sys_path(simple_de
)
assert list(maybe.problems) == [
DependencyProblem(
code='sys-path-cannot-compute',
code='sys-path-cannot-compute-value',
message="Can't update sys.path from f'{name_2}' because the expression cannot be computed",
source_path=Path('sys-path-with-fstring.py'),
start_line=4,
Expand All @@ -211,7 +211,7 @@ def test_dependency_resolver_raises_problem_for_non_inferable_sys_path(simple_de
end_col=27,
),
DependencyProblem(
code='sys-path-cannot-compute',
code='sys-path-cannot-compute-value',
message="Can't update sys.path from name because the expression cannot be computed",
source_path=Path('sys-path-with-fstring.py'),
start_line=7,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/source_code/test_notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,4 @@ def test_raises_advice_when_dbutils_notebook_run_is_too_complex() -> None:
linter = DbutilsLinter(CurrentSessionState())
advices = list(linter.lint(source))
assert len(advices) == 1
assert advices[0].code == "dbutils-notebook-run-dynamic"
assert advices[0].code == "notebook-run-cannot-compute-value"

0 comments on commit b11ac05

Please sign in to comment.