-
Notifications
You must be signed in to change notification settings - Fork 87
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
Table migration mapping #525
Conversation
if re.match(r"^\w+$", default_catalog): | ||
break | ||
else: | ||
print(f"{default_catalog} is not a valid catalog name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make a logger.warn instead and provide a hint for what's a "valid catalog name"
mapping = "element,target,notes\n" | ||
mapping += f"<root>,{default_catalog},Default Mapping created by UCX Install\n" | ||
path = f"{self._install_folder}/table_upgrade_map.csv" | ||
self._ws.workspace.upload(path, mapping.encode("utf8"), format=ImportFormat.AUTO, overwrite=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add some test to validate that we've indeed saved the catalog name
@@ -778,6 +799,11 @@ def _get_ext_hms_conf_from_policy(cluster_policy): | |||
spark_conf_dict[key[11:]] = cluster_policy[key]["value"] | |||
return instance_profile, spark_conf_dict | |||
|
|||
def _write_mapping_conf_file(self, default_catalog: str): | |||
mapping = "element,target,notes\n" | |||
mapping += f"<root>,{default_catalog},Default Mapping created by UCX Install\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand the scope of this PR, why would we write a mapping file when the user can only enter the default catalog name ?
Closes #304