Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dedicated advice code for uncomputed values #2019

Merged
merged 1 commit into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
Loading