change social credibility #42
Conversation
There was a problem hiding this comment.
En resumen,
- Revisen cada comentario que les dejé en el PR
- Un remplazo a
requestesaxios, de hecho eso es lo que usamos en el cliente que está en esta misma organización y nos ha ido bastante bien con ese. - No lo puse en los comentarios, pero todas las llamadas al nuevo web-service que estuvieron desarrollados debería estar en otro módulo que no sea el de calcular la credibilidad (solo las llamadas). Veo que tienen llamadas
createTwitterUser.php,createTweet.php. Esto para poder realizar cambios más fácilmente, supongamos que usan 10 veces el endpoint de createTweet.php en 10 lugares distintos. Si queremos modificar esa llamada pq por ejemplo cambió el URL, habría que ir uno por uno. Si tenemos esas llamadas abstraídas con una funcion, pues los refactors van a ser más cómodos. Este nuevo módulo pudiera estar en/src/web-service/(no usen web-service, pongan un nombre que de a entender que hace exactamente su web-service (entiendo que crea tweets y usuarios, pupdiera sertwitter-offline? idk). - Definir modelos para interactuar con los requests/response de su web-service. guiense de como lo hicimos en
./src/calculator/models.ts. Estos modelos pudieran estar en su carpetica nueva donde tendrán las nuevas llamadas. - Traten de añadir pruebas unitarias para probar que se hacen las llamadas a su web-services. En este backend no hace falta probar que su web-service está funcionando bien, interesa probar que bajo ciertos inputs y outputs obtengamos los resultados deseados. Les recomiendo ver como mockeamos la api de twitter para que ustedes hagan lo equivalente con sus llamadas.
Quedo atento y disculpen la mega demora en atender el PR.
| "request": "^2.88.2", | ||
| "sync-request": "^6.1.0", |
There was a problem hiding this comment.
https://www.npmjs.com/package/request ese package fue deprecado. Integren otro, recomiendo axios
| 'userAPIid':'' | ||
| } | ||
|
|
||
| var Request = require('request') |
There was a problem hiding this comment.
deprecado. Usen otra opción (ver mi comentario del package.json)
| var userTW={ | ||
| 'id_twitter':'', | ||
| 'joined_date':'', | ||
| 'user_name':'', | ||
| 'following':0, | ||
| 'followers':0, | ||
| 'link':'', | ||
| 'location':'', | ||
| 'verified':'', | ||
| 'extraction_method':'1', | ||
| 'followers_impact':0, | ||
| 'user_credibility':0 | ||
| } | ||
|
|
||
|
|
||
| var tweetTW={ | ||
| 'id_tweet_api':'', | ||
| 'text':'', | ||
| 'bad_words_filter':'', | ||
| 'spam_filter':'', | ||
| 'misspelling_filter':'', | ||
| 'extraction_method':'1', | ||
| 'tweet_credibility':'', | ||
| 'retweets':'', | ||
| 'favorites':'', | ||
| 'replies':'', | ||
| 'tweet_url':'', | ||
| 'type':'', | ||
| 'userAPIid':'' | ||
| } |
There was a problem hiding this comment.
No veo por qué debeoms tener estas variables globales, se presta demasiado a race conditions por como express maneja de manera concurrente las requests.
Además, no están aprovechando el tipado de typescript. Este proyecto va a pasar por bastantes personas y mientras más robusto esté mejor. Deberíán definir los modelos (como hacemos ya en /src/calculator/models.ts que correspondan con lo que esperan manipular ustedes.
| await Request.post({ | ||
| 'headers': { 'content-type': 'application/json' }, | ||
| 'url': 'http://107.170.90.209//web-services/services/createTwitterUser.php', | ||
| 'body': JSON.stringify(userTW) | ||
| },function optionalCallback(err:string, httpResponse:string, body:boolean) { | ||
| if (err) {console.error('upload failed:', err)} | ||
| if(body){ getUserTweets(userTW.user_name, userTW.id_twitter)} | ||
| }) |
There was a problem hiding this comment.
Request deprecado. Usen otra lilbrería
| tweetTW={ | ||
| 'id_tweet_api':element.id_str, | ||
| 'text':element.text, | ||
| 'bad_words_filter':'0', | ||
| 'spam_filter':'0', | ||
| 'misspelling_filter':'0', | ||
| 'extraction_method':'1', | ||
| 'tweet_credibility':'0', | ||
| 'retweets':element.retweet_count, | ||
| 'favorites':element.favorite_count, | ||
| 'replies':'0', | ||
| 'tweet_url':media, | ||
| 'type':type, | ||
| 'userAPIid': userId //element.user.id_str | ||
| } |
There was a problem hiding this comment.
mismo comentario de las variables globales
| // if (typeof element.entities.media === 'undefined') { | ||
| // media='' | ||
| // type='' | ||
| //}else{ | ||
| // media=element.entities.media[0].media_url | ||
| // type=element.entities.media[0].type | ||
| //} |
There was a problem hiding this comment.
código comentado -> código a borrar
| @@ -403,7 +403,7 @@ describe('Input Validation', () => { | |||
| errorMessage: 'customValidation.WEIGHT_TEXT_CRED_NOT_EQUALS_TO_1', | |||
| userErrorMessage: 'customValidation.WEIGHT_TEXT_CRED_NOT_EQUALS_TO_1', | |||
| validationCode: 'customValidation.WEIGHT_TEXT_CRED_NOT_EQUALS_TO_1' } ] | |||
| }, { tweetId: 867412932409282560, maxFollowers: 2000, weightSpam: 0, weightBadWords: -1, weightMisspelling: 0, weightText: 0, weightUser: 0.5, weightSocial: 0.5}) | |||
| }, { tweetId: 1244647948870455298, maxFollowers: 2000, weightSpam: 0, weightBadWords: -1, weightMisspelling: 0, weightText: 0, weightUser: 0.5, weightSocial: 0.5}) | |||
There was a problem hiding this comment.
Dado que este tweetId tiene pinta de que va a cambiar bastante frecuente lo mejor es que definamos alguna constante global en este archivo que tenga ese valor y que se use en lugar de pegar el valor a cada rato, así solo se cambia en un solo lado
| Request.post({ | ||
| 'headers': { 'content-type': 'application/json' }, | ||
| 'url': 'http://107.170.90.209//web-services/services/createTweet.php', | ||
| 'body': JSON.stringify( | ||
| tweetTW | ||
| ) | ||
| }) |
There was a problem hiding this comment.
Veamos cuantas llamadas http estamos haciendo aquí en total:
- 1 a la api de twitter en la línea 295
- Por cada tweet en esa respuesta, hacemos otra llamada a su nuevo web-service. Supongamos que son
ntweets.
En total tendríamos n+1 llamadas, el throughput que esto va a generar va a ser que la aplicación sea bastante lenta, y no nos podemos dar ese lujo.
Como lo podemos resolver?
Modifiquen el endpoint de su web-service de crear tweets para que reciba una lista de tweets, haciendo eso, solo tendríamos dos llamadas
- 1 para obtener los tweets del lusuario con la api de twitter
- 1 para POSTear todos los tweets al mismo tiempo a su web-service.
No description provided.