search

spaghetti-open-data / twitAntonio

TweetYourMep fork for the Italian 2013 Elections
http://www.twitantonio.it
GNU Affero General Public License v3.0
16 stars 10 forks source link

closes #18 - Avatar import #28

Closed gaspa closed 11 years ago

paolomainardi commented 11 years ago

Davvero ottimo lavoro!

paolomainardi commented 11 years ago

Ciao Andrea, riaprio questa issue perchè con il dataset di adesso di 680 candidati ieri non riuscito ad importare correttamente (in locale) errore EMFILE: too many files opened, inoltre non credo che il flusso async tra Mongoose e Request lo stiamo gestendo in maniera corretta.

Ho fatto delle prove con la libreria "async" per cercare di rendere i processi separati, ma non ho avuto grandi risultati (considerando gli orari dei commit :)).

Puoi vedere qui: https://github.com/spaghetti-open-data/twitAntonio/blob/master/import/import-avatar.js#L39

gaspa commented 11 years ago

avevo visto, ora cerco di sistemare

gaspa commented 11 years ago

io credo di avere problemi con l'import, invece, dalla shell di mongo:

db.deps15s.find().length() 13 mentre l'import-deps mi aveva detto correttamente 600 e oltre.

paolomainardi commented 11 years ago

Hai provato ad usare una collezione pulita ?

gaspa commented 11 years ago

Fatto: use twitAntonio db.dropDatabase() poi rifatto l'import, e mi dice:

db.deps15s.find().length() 14

paolomainardi commented 11 years ago

@gaspa prova cosi:

setTimeout( function () { mongoose.disconnect(); }, 1000);

Porta quel timeout ad un valore molto più alto, anche qui stessi problemi non è correttamente gestita l'async di nodejs probabilmente.

gaspa commented 11 years ago

già, giusto. (che poi, me n'ero accorto per gli avatar... che pistola) Ok, finisco dopo cena, a questo punto, scappo e a piu' tardi.

gaspa commented 11 years ago

(pero' l'import degli avatar ora me l'ha fatto) $ ls public/avatars/* | wc -l 694 pure troppi. :P

paolomainardi commented 11 years ago

Ok, allora poi rifacciamo anche l'altro import usando la stessa tecnica!

gaspa commented 11 years ago

confermo, messo a 10000 il timeout in import-deps e mi ha rifatto correttamente tutta la procedura, qui. Anche in import-avatar, in realtà con un timeout abbastanza lungo non mi da' problemi. Da' questo warning: (node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit. ma non dovrebbe dare problemi. (anche se potremmo poi toglierlo :P )

Se puoi fare due prove anche tu, se va tutto bene, poi toglierei async e alzerei i timeout, semplicemente. (una dipendenza in meno, no? )

dottorblaster commented 11 years ago

Ho committato al volo l'estensione del timeout, spero non vi dispiaccia :)

dottorblaster commented 11 years ago

https://github.com/spaghetti-open-data/twitAntonio/commit/5c00c6473a9255d859c873952757d0c87d4c2ac3 Impostato il timeout a 10000 per import-deps

dottorblaster commented 11 years ago

(Ragazzi stavo guardando al volo, magari sono un niubbone io ma in https://github.com/spaghetti-open-data/twitAntonio/blob/avatar_import/import/import-avatar.js mancano un paio di puntevvirgola o sbaglio?

paolomainardi commented 11 years ago

Non è un problema di sintassi, ma proprio del meccanismo asincrono di nodejs, che non stiamo effettivamente gestendo con questi importer, ed è per questo che l'unica soluzione valida ora è un timeout alto. Bisognerebbe riscriverli appena si ha un po' di tempo.

dottorblaster commented 11 years ago

No no, lo dicevo non in relazione al problema in sé ma proprio perché secondo me mancano :D

Comunque https://github.com/caolan/async qui la documentazione di async, magari ci smanetto io in serata (anche se sotto esame non sto dando il meglio)

paolomainardi commented 11 years ago

@dottorblaster se sei sotto esame concentrati su quello (la paternale la dovevo fare!). Async secondo me possiamo anche evitarlo, l'avevo usato ieri giusto per riuscire ad abozzare qualcosa di veloce, ma non credo sia strettamente necessario, ma se vuoi provarci è sicuramente d'aiuto!

dottorblaster commented 11 years ago

Supporto sperimentale ad async in questo commit, branch dedicato: https://github.com/spaghetti-open-data/twitAntonio/commit/5d83d96862871dab587436fac80dc63bef3ba226

Lo provate e mi dite se funziona? Dovrebbe andare.

gaspa commented 11 years ago

Ora provo, ma a me funziona anche senza async,come dicevo sopra. Forse la soluzione sarebbe proprio evitare di fare tutto in parallelo. Non ho ancora conoscenze approfondite di nodejs, quindi non saprei come fare, ma mi fa strano proprio che la disconnessione dal DB debba essere fatta in questo modo. Non avrebbe senso fare solo un tot di connessioni in parallelo?

dottorblaster commented 11 years ago

Beh esiste anche il ForEachSeries :) Il punto è proprio cercare di fare le cose in maniera asincrona, node è concepito per questo e per quanto possa essere un virtuosismo io cercherei di sfruttarlo (anche perché, tanto per, io sto scrivendo codice per imparare e quindi metto mano dove posso)

2013/1/28 Andrea Gasparini notifications@github.com

Ora provo, ma a me funziona anche senza async,come dicevo sopra. Forse la soluzione sarebbe proprio evitare di fare tutto in parallelo. Non ho ancora conoscenze approfondite di nodejs, quindi non saprei come fare, ma mi fa strano proprio che la disconnessione dal DB debba essere fatta in questo modo. Non avrebbe senso fare solo un tot di connessioni in parallelo?

— Reply to this email directly or view it on GitHubhttps://github.com/spaghetti-open-data/twitAntonio/pull/28#issuecomment-12803838.

gaspa commented 11 years ago

Right,pero' bisognerebbe trovare il modo di trovare una callback che esegua alla fine di tutte le altre operazioni, un po' tipo "join" dei thread.

Poi: ho provato e funziona, ma a me andava anche prima, non so quanto sia significativo.

dottorblaster commented 11 years ago

È notevolmente significativo, significa che ho scritto bene l'async al primo colpo :D Il modo di fare la callback c'è, è quell'ultima funzionzina che per ora è vuota ché non mi serviva. Come "join"... non so, per ora fa quello che deve :P se mi vengono idee durante la notte implemento.

Ale

paolomainardi commented 11 years ago

Gia che c'ero ho riscritto anche io la mia versione asincrona, sembra funzionare correttamente: https://github.com/spaghetti-open-data/twitAntonio/commit/4994d00138dd138a149f72798bb285c49c20395c

E' un tripudio di callback, ma questo è nodejs purtroppo (e non che è che sia un mago di questo ambiente :))

nelsonmau commented 11 years ago

@paolomainardi vai a dormire! ;)

Uè ho cambiato la pagina help ma non vedo la nuova. Why?

Gia che c'ero ho riscritto anche io la mia versione asincrona, sembra funzionare correttamente: 4994d00https://github.com/spaghetti-open-data/twitAntonio/commit/4994d00138dd138a149f72798bb285c49c20395c

E' un tripudio di callback, ma questo è nodejs purtroppo (e non che è che sia un mago di questo ambiente :))

— Reply to this email directly or view it on GitHubhttps://github.com/spaghetti-open-data/twitAntonio/pull/28#issuecomment-12812656.

paolomainardi commented 11 years ago

Mo te la sbatto in produzione, non è automatico il passaggio dal commit su github al codice che gira sul server.

Hai ragione, pullo e vado a letto :)

2013/1/29 nelsonmau notifications@github.com

@paolomainardi vai a dormire! ;)

Uè ho cambiato la pagina help ma non vedo la nuova. Why?

  • sorry for typos, i'm from mobile - Il giorno 29/gen/2013 00:52, "Paolo Mainardi" notifications@github.com ha scritto:

Gia che c'ero ho riscritto anche io la mia versione asincrona, sembra funzionare correttamente: 4994d00< https://github.com/spaghetti-open-data/twitAntonio/commit/4994d00138dd138a149f72798bb285c49c20395c>

E' un tripudio di callback, ma questo è nodejs purtroppo (e non che è che sia un mago di questo ambiente :))

— Reply to this email directly or view it on GitHub< https://github.com/spaghetti-open-data/twitAntonio/pull/28#issuecomment-12812656>.

— Reply to this email directly or view it on GitHubhttps://github.com/spaghetti-open-data/twitAntonio/pull/28#issuecomment-12812729.

[image: TWINBIT logo] http://twinbit.it/

Paolo Mainardi Twinbit Founder - CTO www: twinbit.it mobile: (+39) 3401678089 skype: paolo_mainardi linkedin: paolomainardi http://www.linkedin.com/in/paolomainardi twitter: @paolomainardi http://twitter.com/paolomainardi blog: paolomainardi.com

nelsonmau commented 11 years ago

Occhio che ho cambiato la pagina Aiuto. Ora si chiama "Sei un candidato? Aggiungi il tuo nome". Ma non ho capito come funziona l'header qui, cioè se si eredita o no in tutte le pag...

@gaspa la cosa che ti dicevo oggi? hai avuto tempo di guardarci? io c'ho pensato su si potrebbe fare così: "Sono presenti {sum} account twitter su {totale} candidati verificati". La value totale la puoi otterenere facendo 'concatenate' tra {candidato_circoscrizione}.

paolomainardi commented 11 years ago

Ecco qua: http://www.twitantonio.it/ @nelsonmau No quella parte è in ogni pagina, quindi vanno ripetute le modifiche, le parti comuni le trovi dentro "includes/header.ejs-footer.ejs"

nelsonmau commented 11 years ago 
ah ecco, mica potevo concludere la giornata senza una lamerata!Il 29/01/2013 01:06, Paolo Mainardi ha
  scritto:

  Ecco qua: http://www.twitantonio.it/@nelsonmau No quella parte è in ogni
    pagina, quindi vanno ripetute le modifiche, le parti comuni le
    trovi dentro "includes/header.ejs-footer.ejs"

    —
    Reply to this email directly or view
      it on GitHub. 

--

Andrea Nelson Mauro - Datajournalist

http://www.dataninja.it http://twitter.com/nelsonmau

gaspa commented 11 years ago

@paolomainardi @dottorblaster ho visto che c'è ancora 1000 nel timeout del primo import. Intanto che lavorate ad async lo pusho un po' piu' alto.