sadpandajoe commented on code in PR #43188:
URL: https://github.com/apache/superset/pull/43188#discussion_r3818865298
##########
superset/utils/file.py:
##########
@@ -32,5 +37,13 @@ def sanitize_title(title: str) -> str:
def get_filename(model_name: str, model_id: int, skip_id: bool = False) -> str:
model_name = sanitize_title(model_name)
slug = secure_filename(model_name)
- filename = slug if skip_id else f"{slug}_{model_id}"
- return filename if slug else str(model_id)
+ suffix = "" if skip_id else f"_{model_id}"
+ # The name goes into a ZIP entry that already carries an
+ # `<asset>_export_<timestamp>/<type>/` prefix and a `.yaml` suffix, and the
+ # user's own extraction directory sits in front of all of it. A chart
titled
+ # with a couple of hundred characters therefore produced an entry Windows
+ # refuses to extract, even though the archive itself was written fine. Trim
+ # the slug rather than the id: the id is what keeps two similarly titled
+ # assets from colliding inside one archive.
+ slug = slug[: max(MAX_FILENAME_LENGTH - len(suffix), 0)].rstrip("._-")
Review Comment:
Agreed—two distinct long database names that share the first 200 characters
now produce the same archive path, and the export's seen set drops one YAML
file. Could the truncated `skip_id=True` path retain a deterministic
disambiguator?
##########
superset/utils/file.py:
##########
@@ -32,5 +37,13 @@ def sanitize_title(title: str) -> str:
def get_filename(model_name: str, model_id: int, skip_id: bool = False) -> str:
model_name = sanitize_title(model_name)
slug = secure_filename(model_name)
- filename = slug if skip_id else f"{slug}_{model_id}"
- return filename if slug else str(model_id)
+ suffix = "" if skip_id else f"_{model_id}"
+ # The name goes into a ZIP entry that already carries an
+ # `<asset>_export_<timestamp>/<type>/` prefix and a `.yaml` suffix, and the
+ # user's own extraction directory sits in front of all of it. A chart
titled
+ # with a couple of hundred characters therefore produced an entry Windows
+ # refuses to extract, even though the archive itself was written fine. Trim
+ # the slug rather than the id: the id is what keeps two similarly titled
+ # assets from colliding inside one archive.
+ slug = slug[: max(MAX_FILENAME_LENGTH - len(suffix), 0)].rstrip("._-")
+ return f"{slug}{suffix}" if slug else str(model_id)
Review Comment:
Agreed—this now maps a short trailing-hyphen name to the same archive path
as the name without the hyphen, so one export is dropped. Could the separator
cleanup run only when truncation occurred?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
