flask 'in memory zip create' (stringIO, zipfile)

Django, Flask, Bottle, WSGI, CGI…
Antworten
s45f
User
Beiträge: 12
Registriert: Donnerstag 27. Februar 2014, 13:47

Hallo zusammen,

ich hoffe ihr könnt mir nochmal helfen.
Und zwar möchte ich dem user anbieten größere Mengen an Daten als zip zu downloaden.
Und dieses Zipfile möchte ich gerne nicht auf die Platte schreiben sondern im RAM zusammenbasteln und von dort aus an den user senden.
Das ganze klappt auch schon so halb, allerdings sind die gezippten Daten leer.

Edit:

Der Übersicht halber hab ich das mal zusammengefasst (und hoffentlich nichts wichtiges vergessen)
darunter ist dann noch mal die ganze funktion in der ich es benutzte.

Zusammenfassung:

Code: Alles auswählen

import StringIO
import zipfile

in_memory_zip = StringIO.StringIO()
zf = zipfile.ZipFile(in_memory_zip, "a", zipfile.ZIP_DEFLATED, False)
for file in list:
    zf.writestr(filename, file)
for zfile in zf.filelist:
    zfile.create_system = 0
zf.close()
in_memory_zip.seek(0)
in_memory_zip.getvalue()
return send_file(in_memory_zip, attachment_filename=attachment_name, as_attachment=True)

Code: Alles auswählen

import StringIO
import zipfile

@app.route('/download_pictures_as_zipfile')
def dl_as_zip():
in_memory_zip = StringIO.StringIO()
    zf = zipfile.ZipFile(in_memory_zip, "a", zipfile.ZIP_DEFLATED, False)
    x_list = []
    _list = []
    db_list = []
    if 'idlist' in session:
        _list = session['idlist']
    else:
        return render_template('fail.html')
    conn = engine.connect()
    db_session = Session(bind=conn)
    for elem in _list: # prevent removing first item in session cookie on every reload
        x_list.append(elem)
    attachment_name = 'lxm_'+x_list[0]+'.zip'
    del x_list[0]
    for item in x_list:
        get_data = select([Pictures], Pictures.data_id == item)
        data = conn.execute(get_data)
        db_list.append([])
        for elem in data:
            db_list[-1].append(elem.storename)
            db_list[-1].append(elem.sha1)
            db_list[-1].append(elem.extension)
    db_session.close()
    for elem in db_list:
#        print os.path.isfile(os.path.join(app.config['DOWNLOAD_FOLDER'],elem[1]+'.'+elem[2]))
        file_to_zip = os.path.join(app.config['DOWNLOAD_FOLDER'],elem[1]+'.'+elem[2])
        zi.writestr(elem[0],file_to_zip)
    for zfile in zf.filelist:
        zfile.create_system = 0
    zf.close()
    in_memory_zip.seek(0)
    in_memory_zip.getvalue()
    return send_file(in_memory_zip, attachment_filename=attachment_name, as_attachment=True)
Zugegebener maßen ist dies mein erster Ausflug mit StringIO und Zipfile, sollte ich da irgendwie totalen bullshit gemacht haben bitte ich um Nachsicht.
s45f
User
Beiträge: 12
Registriert: Donnerstag 27. Februar 2014, 13:47

So läufts:

Code: Alles auswählen

in_memory_zip = StringIO.StringIO()
zf = zipfile.ZipFile(in_memory_zip, "a", zipfile.ZIP_DEFLATED, allowZip64=False) 
for file in list:
    zf.writestr(filename, file)
    in_memory_zip.getvalue()
for zfile in zf.filelist:
    zfile.create_system = 0
zf.close()
in_memory_zip.seek(0)
return send_file(in_memory_zip, attachment_filename=attachment_name, as_attachment=True)
Frisst allerdings zur Zeit noch sehr viel performance.
Sirius3
User
Beiträge: 17754
Registriert: Sonntag 21. Oktober 2012, 17:20

@s45f: »list« ist eine wichtige Builtin-Funktion und sollte nie als Variablennamen benutzt werden. Ebenso »file«. Was glaubst Du bewirkt Zeile 5? Und warum willst Du nur die Dateinamen zippen, oder was enthält jetzt »file«? Den ganzen Dateiinhalt?
BlackJack

@s45f: Kannst Du auch erklären warum es jetzt funktioniert und vorher nicht? Ansonsten fällt das in den Bereich Vodoo und man muss ganz fest daran glauben dass es tatsächlich funktioniert und nicht irgendwann wieder falsche Daten liefert. Die `getvalue()`-Aufrufe sind zum Beispiel abenteuerlich IMHO. Was soll das? Ist Dir klar was das für Laufzeit und Speicherverbrauch bedeutet?

Die Namen in der Funktion sind teilweise zu kurz und nichtssagend, und was dort mit den Listen angestellt wird, ist reichlich umständlich.

Der Test ob es überhaupt eine 'idlist' in der `session` gibt, sollte am Anfang der Funktion stehen. Es macht keinen Sinn irgend etwas für das Ergebnis vorzubereiten, wenn man dann zwischendurch mal testet ob das Ergebnis überhaupt erstellt werden kann. Insgesamt sind die einzelnen Schritte unnötig stark vermischt. Listen werden beispielsweise am Anfang erstellt obwohl einige erst viel später benötigt werden, oder das In-Memory-ZipFile wird erstellt bevor die Datenbank nach den Dateinamen befragt wird. Das macht die ganze Funktion sehr unübersichtlich.

Die Namen `_list`, `x_list`, und `db_list` sind viel zu generisch. Ein Name soll dem Leser verraten was der Wert dahinter bedeutet. Wenn eine Liste IDs enthält, dann sollte man sie besser `ids` nennen statt `_list`.

`x_list` lässt sich komplett einsparen in dem man die Liste einfach nicht verändert, sondern über eine Kopie ohne das erste Element iteriert. Was mit der Slicing-Notation ganz simpel ist: ``copy_without_first_element = a_list[1:]``.

`db_list` wird extrem umständlich befüllt. Wie kommt man auf die Idee eine leere Liste anzuhängen um dann die dann zu befüllen in dem man das letzte Element der `db_list` in jeweils einer Quelltextzeile um ein weiteres Element erweitert, statt gleich eine Liste mit den drei Elementen anzuhängen‽ Zumal die Schleife über `data` ja nur einmal durchlaufen wird, und Schleifen von denen man weiss das sie immer nur einmal durchlaufen werden, sind sinnfrei.

Den Sinn von `db_list` würde ich aber generell in Frage stellen. Es werden von der Datenbank `Pictures`-Objekte abgefragt (die Klasse sollte wohl eigentlich `Picture` heissen), um dann benannte Attribute abzufragen und die zu anonymen Listenelementen zu machen. Warum? Der Zwischenschritt ist unnötig und macht den weiteren Quelltext schwerer verständlich.

Ungetestet:

Code: Alles auswählen

@app.route('/download_pictures_as_zipfile')
def download_as_zip():
    ids = session.get('idlist')
    if ids is None:
        return render_template('fail.html')

    in_memory_zip = StringIO()
    with closing(ZipFile(in_memory_zip, 'a', ZIP_DEFLATED, False)) as zip_file:
        with closing(Session(bind=engine.connect())) as db_session:
            for item in ids[1:]:
                for picture in db_session.execute(
                    select([Pictures], Pictures.data_id == item)
                ):
                    filename = os.path.join(
                        app.config['DOWNLOAD_FOLDER'],
                        picture.sha1 + '.' + picture.extension
                    )
                    zip_file.writestr(picture.storename, filename)
                    
        for zfile in zip_file.filelist:
            zfile.create_system = 0
    
    in_memory_zip.seek(0)
    return send_file(
        in_memory_zip,
        attachment_filename='lxm_' + ids[0] + '.zip',
        as_attachment=True
    )
DasIch
User
Beiträge: 2718
Registriert: Montag 19. Mai 2008, 04:21
Wohnort: Berlin

Ich gehe mal davon aus dass engine und session hier von SQLAlchemy kommen. Diese sollte man so allerdings nicht nutzen wie du es hier tust, wie man auch der SQLAlchemy Dokumentation entnehmen kann. Die übrigens für Flask Anwendungen "strongly recommends" Flask-SQLAlchemy zu nutzen.

Engine ist ein Pool von Verbindungen zur Datenbank, den sollte man einmal global für jede Datenbank bzw. einmal für jede Flask Anwendung und Datenbank erstellen. Für die session sollte man in diesem Kontext eine scoped_session nutzen, wir ebenfalls detailiert in der Dokumentation erklärt.
s45f
User
Beiträge: 12
Registriert: Donnerstag 27. Februar 2014, 13:47

@Sirius3: stimmt, ich habs nur kurz runtergebrochen um es verständlicher zu machen daher hab ich für die liste einfach list gewählt, in der langen version stehen dann auch die 'korrekten namen'

@Blackjack: danke für die code verbesserung, liest sich ungleich besser und eleganter.
hab ich gleich adaptiert.

Wieso ist die getvalue() abenteuerlich?
Bzw. ja mir ist klar geworden dass die funktion nen performance killer ist.
Leider hab ich keine idee wie man das ändern kann daher, hab ich jetzt einfach die hardware 'verbesser' (läuft inner vmware)
wäre mir natürlich schon lieber wenn es besser wäre und nicht so viel systemlast erzeugen würde.
Allerdings muss ich hier passen da mir schlicht die erforderlichen Kentnisse fehlen um das ganze schonender laufen zu lassen.

Mit den ganzen listen und co muss ich dir recht geben, da sollte ich etwas mehr Zeit rein investieren den vernünftige Namen zu geben.

@DasIch: Ehrlich gesagt hab ich das SQLAlchemy eher durch raten hinbekommen, da es doch verteufelt kompliziert ist, gerade wenn man das erste mal etwas macht.

Also ich hab
dev-python/flask-sqlalchemy installiert, allerdings bin ich mir nicht sicher wie das mit der Benutzung aussieht:
http://flask.pocoo.org/docs/patterns/sqlalchemy/
erwähnt das flask sqalchemy zB überhaupt nicht
allg. empfinde ich die import orgie bei sqlalchemy gerade für mich als anfänger als unübersichtlich.

Code: Alles auswählen

from sqlalchemy.orm import sessionmaker
from sqlalchemy import create_engine, desc
from sqlalchemy.sql import select, and_
why?

Wenn ihr noch irgendwie tipps oder derart habt wie man den systemload beim erstellen der dateien besser in griff bekommt, oder man das problem ggf. anders angehen kann ...
BlackJack

@s45f: `getvalue()` erzeugt bei dir in jedem Schleifendurchlauf eine Zeichenkette mit dem kompletten Inhalt der „Datei”. Da sich die davor in dem Schleifenkörper verändert, wird jedes mal eine Kopie von den Daten in der „Datei” erstellt, die bei jedem Schleifendurchlauf grösser wird. Und was machst Du dann mit dieser Kopie der Daten? Gar nichts! Die wird einfach nur erstellt, Speicher wird angefordert, Daten werden kopiert, und dann wird das alles wieder freigegeben. Das finde ich abenteuerlich, weil ich den Sinn von dieser Aktion nicht nachvollziehen kann.

Ich glaube ich würde an der Stelle auch nicht `send_file()` verwenden. Die Funktion ist nützlich damit im besten Fall der Webserver eine Datei direkt ausliefert, oder falls das nicht geht, diese Funktion zumindest sicherstellt, dass die Datei die darüber ausgeliefert wird nicht komplett in den Speicher gelesen werden muss, sondern von dem Dateiobjekt „gestreamt” werden kann. Da die Daten in diesem Fall sowieso schon komplett im Speicher sind, fallen diese beiden Eigenschaften weg und man könnte die Daten auch direkt mit dem passenden MIME-Typ versehen ausliefern. IMHO.
s45f
User
Beiträge: 12
Registriert: Donnerstag 27. Februar 2014, 13:47

@BlackJack:

Achso,
ich dachte eigentlich das er einfach nen zipfile im speicher aufmacht und die einzelnen Daten direkt dort rein appendet durch die Schleife bis das abgearbeitet ist.
Hab nach geschaut ob es ggf. möglich wäre alles erstmal in nen string zu schreiben und den an zipfile zu übergeben. Um am Ende das nur 1x machen zu müssen.
Allerdings wird das nicht unterstützt
Bin gerade endlich dazu gekommen deine Variante zu testen, das Problem dabei ist das die ganzen Daten leer sind, daher kam auch mein 'getvalue()' da es das Problem gelöstet hatte.
zipinfo /tmp/lxm_AU102569.zip
Archive: /tmp/lxm_AU102569.zip
Zip file size: 12146 bytes, number of entries: 53
?rw------- 2.0 fat 116 b- defN 14-Mar-11 09:49 18284-00422_300.jpg
?rw------- 2.0 fat 116 b- defN 14-Mar-11 09:49 20916-00203_300.jpg
[...]
53 files, 6148 bytes uncompressed, 5108 bytes compressed: 16.9%
BlackJack

@s45f: Meine Variante schreibt als Dateiinhalt den Dateinamen. Und zwar weil Dein erster Quelltext, auf dem das basiert, das auch so gemacht hat. Sorry das ich das nicht korrigiert habe.
s45f
User
Beiträge: 12
Registriert: Donnerstag 27. Februar 2014, 13:47

Kann dir gerade nicht ganz folgen,
so ist es doch auch vorgesehen.

Code: Alles auswählen

 |  writestr(self, zinfo_or_arcname, bytes, compress_type=None)
 |      Write a file into the archive.  The contents is the string
 |      'bytes'.  'zinfo_or_arcname' is either a ZipInfo instance or
 |      the name of the file in the archive.

Edit:

Was du wohl meintest

Code: Alles auswählen

|  write(self, filename, arcname=None, compress_type=None)
|      Put the bytes from filename into the archive under the name
|      arcname.
Habs jetzt abgeändert das er

Code: Alles auswählen

[...]
#                    zip_file.writestr(picture.storename, str(filename))
                    zip_file.write(filename, arcname=picture.storename)
[...]
macht.

Jetzt läuft es auch wie erhofft, gibt es irgendwas was dagegen spricht es so zu lassen?
Zuletzt geändert von s45f am Dienstag 11. März 2014, 10:29, insgesamt 1-mal geändert.
Benutzeravatar
/me
User
Beiträge: 3556
Registriert: Donnerstag 25. Juni 2009, 14:40
Wohnort: Bonn

s45f hat geschrieben:Kann dir gerade nicht ganz folgen,
so ist es doch auch vorgesehen.
Nein, ist es nicht. Du interpretierst das falsch. Als Parameter für bytes sind die Daten als solche vorgesehen, nicht ein Dateiname.
Antworten