Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNIB REST: minor refactor #522

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nobeeakon
Copy link
Contributor

  • Reemplaza string concatenation con el uso de la clase URL

  • Quita Promises alrededor de knex, que ya esta regresando una promise

  • Quita lodash, que solo se usaba para .forEach, usando ahora el metodo de array.forEach

  • Cambia el timeout que estaba en 10 minutos a 30 segundos, no vi algún endpoint que pudiera tardar tanto, por lo que entiendo regresan una cantidad limitada de resultados

  • cosas menores:

    • algunos errores de dedo o a copy pasta,
    • algunas rutas tenian errores leves (por ej. al usar "grupo" se estaba agregando otro queryParam 'especie_id=' , o /espeice/{id}/ancestry agregaba /arbol_nodo_inicial luego de definir los queryParams ),
    • algunas validaciones se cambian de string a number

@nobeeakon
Copy link
Contributor Author

Al parecer el timeout estaba relacionado con la búsqueda por región. Agregué un issue #523 , lamentablemente no sé de ruby/rails así que no puedo ayudar mucho ahí

@nobeeakon
Copy link
Contributor Author

hmm, igual podria tratar de apoyar en otras cosas, ... intenté echar a andar localmente el sitio pero depende de la base de datos y demás que solamente vienen mencionados en el README pero no vi una clara-reproducible para correrlo localmente :(

@albertoglezba
Copy link
Member

Gracias Daniel, así es es completamente dependiente de la base, este mes trabajare para empaquetar todo y sea fácilmente reproducible.
En cuanto a este PR tendrá que esperar ya que la persona que se encargaba de esta parte ya no labora con nosotros, quedara en la lista de pendientes.
Lo que te puedo decir es que en efecto la parte de nodejs sí requiere una optimización.

Saludos, seguimos en contacto

@nobeeakon
Copy link
Contributor Author

Gracias por la respuesta @albertoglezba , venga, quedo al pendiente entonces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants