ARPA-SIMC / dballe

Fast on-disk database for meteorological observed and forecast data.
Other
19 stars 6 forks source link

using transaction without "with" statment #195

Closed pat1 closed 4 years ago

pat1 commented 5 years ago

Usare le transazioni risulta obbligatorio come al momento risulta da #194 ma non so come utilizzare le transazioni non utilizzando lo statment "with" del python. Questo è necessario se non erro per utilizzare una transazione distribuita ad esempio con inizio e fine in metodi diversi di una classe. Ho provato a utilizzare cur.__exit__(None,None,None) e pare funzionare ma i sui parametri non sono documentati e mi parrebbe più corretto avere un metodo ben documentato della classe transazione di dballe. Essendo l'uso della transazione al momento vincolato da #194 questo mi blocca il porting a dballe8 di borinud

edigiacomo commented 5 years ago

Non credo che attualmente si possa. Non capisco però in quale scenario questa cosa ti limiti, puoi provare a fare un esempio semplificato di una transazione distribuita con inizio e fine in metodi diversi di una classe? Magari si riesce ad aggirare.

pat1 commented 5 years ago

borinud: https://github.com/r-map/rmap/blob/master/python/borinud/utils/source.py

edigiacomo commented 5 years ago

Mi rendo conto che ho dato un contributo significativo a quel codice, ma riesci a fare un esempio semplificato che evidenzi la necessità di iniziare la query in un metodo e finirla in un altro? Oppure indicarmi almeno quali righe di codice sono significative in quel file che a tuo parere non si riescono a tradurre con un with ?

pat1 commented 5 years ago

attualmente la classe astratta DB https://github.com/r-map/rmap/blob/master/python/borinud/utils/source.py#L61 viene poi estesa qui: https://github.com/r-map/rmap/blob/master/python/borinud/utils/source.py#L258 e viene utilizzata qui per creare un iteratore con __iter__ https://github.com/r-map/rmap/blob/master/python/borinud/v1/views.py#L52 e __next__ https://github.com/r-map/rmap/blob/master/python/borinud/v1/views.py#L64

quindi in sostanza per essere polimorfi dall'iteratore dballe se ne fa un altro polimorfo e quindi iter e next sono metodi differenti

edigiacomo commented 5 years ago

Si potrebbe fare una cosa del genere:

class DballeDB(DB):
    def query_stations(self, rec):
        with db.transaction() as tr:
            for station in query_stations(rec):
                yield station.data

E il tuo iterator diventa un iterable (non serve __next__):

class dbajson:
    def __iter__(self):
        if self.summary:
           gen = get_db(dsn=self.dsn,last=self.last).query_summary(self.q)
        elif self.stations:
            gen = get_db(dsn=self.dsn,last=self.last).query_stations(self.q)
        ....
        for result in gen:
            yield result            

Le ultime due righe possono in realtà diventare un semplice yield from gen.

pat1 commented 5 years ago

a parte che bisognerebbe cambiare tutte le classi per l'accesso ai vari DB ho questo problema.

Exception Type: AttributeError 'dballe.CursorStationDB' object has no attribute 'data'

se uso query invece che data poi accedendo ottengo:

Exception Type: KeyError Exception Value: 'ident'

un po' difficile capire come utilizzare query visto che la documentazione dice solo: return a dict with a query to select exactly the current value at this cursor

pat1 commented 5 years ago

query è fatto così:

{'report': 'simnbo',
 'lat': Decimal('43.69223'),
 'lon': Decimal('12.13623'),
 'mobile': False}

è un dizionario puro quindi forse è corretto che non ci sia 'ident' ma mancano anche i metodi enqi etc. ci lavoro ancora ...

edigiacomo commented 5 years ago

Scusami, non mi ero accorto che non c'è l'attributo CursorStationDB.data. Non userei però query perché quello è fatto, appunto, per le query. Non so però se restituire dei Cursor* sia una buona idea, perché non so quanto dipendano dal fatto che la transazione rimanga aperta o meno: finché lavoro col generatore va tutto bene, ma è corretto fare una cosa del tipo list(db.query_stations()) ? Qui chiedo a @spanezz una dritta. Altrimenti, mi sa che tocca creare delle strutture dati ad-hoc.

spanezz commented 4 years ago

Per le transazioni, invece di chiamare __exit__, puoi chiamare .commit() o .rollback(). __exit__ non fa altro che chiamare commit() se tutto è andato bene, o rollback() se è stata sollevata un'eccezione.

spanezz commented 4 years ago

CursorStation e CursorStationDB non hanno .data perché normalmente puntano solo a delle stazioni e non a dei dati. Potete usare cur["station"] per farvi dare l'oggetto dballe.Station puntato dal cursore, coi dati della stazione.

list(db.query_stations()) ti dà una lista con ripetuto N volte lo stesso oggetto cursore, perché iterare un cursore non fa altro che avanzarlo e restituire se stesso.

Non mi è chiaro però cosa dovete fare qui: salvare i dati che escono dal db, ok, e poi per farci cosa?

edigiacomo commented 4 years ago

Credo che @pat1 abbia poi risolto in altro modo, lascio eventualmente chiudere la issue a lui.

pat1 commented 4 years ago

io documenterei commit() e rollback() e chiuderei la issue