radia.create_archive should be download data file?

[robnagler]

@mkeilman there is special code for radia to export the dump file as an archive. This seems like this should be download data file or something similar. Please explain the usage.

[mkeilman]

The dump file is conceptually an archive and should be handled the same way. At least, I think that was my line of thought 3 years ago. I have no idealogical attachments to this argument.

[robnagler]

"Archive" has a specific meaning: a dump of sirepo data for a simulation. A dump file is more like, say, exporting a python file, which is not a complete archive of a simulation. importFile can take python files (only for SRW) and archives. This is asymmetric on purpose.

The essence of this issue are if statements which complicate code and prompt questions like this one. Moreover, by spreading knowledge about the concept of "archive" we have to understand more code to understand the meaning of the word "archive". And, maintenance problems:

def create_archive(sim, qcall):
    """Zip up the json file and its dependencies

    Args:
        sim (PKDict): parsed request

    Returns:
        py.path.Local: zip file name
    """
    if hasattr(sim.template, "create_archive"):
        res = sim.template.create_archive(sim, qcall)
        if res:
            return res
    if not pkio.has_file_extension(sim.filename, "zip"):
        raise sirepo.util.NotFound(
            "unknown file type={}; expecting zip".format(sim.filename)
        )
    with simulation_db.tmp_dir(qcall=qcall) as d:
        f, c = _create_zip(sim, out_dir=d, qcall=qcall)
        return qcall.reply_attachment(f, filename=sim.filename)

Note the comment, which is incorrect. This code can be simplified without the expense of increasing complexity elsewhere, because downloadDataFile already is dispatched. Also note that raising a "NotFound" is incorrect, and leads to more confusion. The response is probably bad request, but it's unnecessary to check the suffix if there's only one type of archive. It's up to the client to pass the correct file name to save.

The simplified code would leave the comment the same as it is now (as it is correct), and be:

    with simulation_db.tmp_dir(qcall=qcall) as d:
        f, c = _create_zip(sim, out_dir=d, qcall=qcall)
        return qcall.reply_attachment(f, filename=sim.filename)

[mkeilman]

I see also that the html case can be excised - _create_html doesn't even exist any more.

I'll pass your comments on to the developer who wrote this section 😉 5c3d41a

[robnagler]

That's why I was looking at the code, because we removed _create_html and there was this cruft.

[robnagler]

Technically the html file wrappped and embedded the zip file so the comment was quasi correct. The code was necessary at the time, I think.