sadpandajoe commented on code in PR #43188:
URL: https://github.com/apache/superset/pull/43188#discussion_r3870952889
##########
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:
The dataset exporter puts a capped database name and a capped dataset name
in the same ZIP member (`datasets/<db>/<dataset>.yaml`), so two 200-character
components still make a roughly 446-character extraction path and Windows
extraction continues to fail. Could the cap account for the complete member
path and add a long database-plus-dataset regression?
--
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]