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

API de metadatos para Field #187

Merged
merged 19 commits into from
Feb 14, 2018
Merged

API de metadatos para Field #187

merged 19 commits into from
Feb 14, 2018

Conversation

lucaslavandeira
Copy link
Contributor

Closes #169

Primera versión de la API de metadatos.

  • Mueve el singleton del cliente de Elasticsearch a libs
  • Instancia una nueva app metadata con un endpoint metadata/search: acepta parámetros q, limit y offset. Los datos se cargan manualmente corriendo el management command index_metadata [datajson_url]

@coveralls
Copy link

coveralls commented Feb 9, 2018

Coverage Status

Coverage increased (+0.8%) to 82.943% when pulling 0ab5288 on metadata-api into 53fc68e on master.

def __init__(self):
self.elastic = ElasticInstance.get()

def index(self, datajson_url):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talvez datajson_url (o un objecto DataJson) debería ser una variable de instancia en vez de andar pasandolo como referencia?

elastic = ElasticInstance.get()
for success, info in parallel_bulk(elastic, actions):
if not success:
print(info)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Podemos usar el loggin default de django? Talvez dejando un método como para una futura implementacion de un mail de "Indexados/fallidos/totales"

actions = []
for field in data_json.get_fields():
if field.get('specialType'):
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Una forma de implementar el for sin el continue? 🤔

offset = self.args[constants.PARAM_OFFSET]
limit = self.args[constants.PARAM_LIMIT]
search = Field.search(using=es_client).query('match', _all=querystring)
search = search[offset:limit + offset]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A elasticsearch no le podés pasar ya el offset y el limit?
https://www.elastic.co/guide/en/elasticsearch/reference/5.6/search-request-from-size.html

o asi se hace con la libreria elasticsearch-dsl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se hace así

@@ -15,5 +15,6 @@
url(r'^django-rq/', include('django_rq.urls')),
url(r'^api/', include('series_tiempo_ar_api.apps.api.urls', namespace="api")),
url(r'^analytics/', include('series_tiempo_ar_api.apps.analytics.urls', namespace='analytics')),
url(r'^metadata/', include('series_tiempo_ar_api.apps.metadata.urls', namespace='metadata')),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Se puede poner la url para que quede como "hermana" de /series/
O sea, /api/metadata/ en vez de /metadata/

@leandro-gomez leandro-gomez merged commit c6a55e5 into master Feb 14, 2018
@lucaslavandeira lucaslavandeira deleted the metadata-api branch February 19, 2018 16:01
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.

None yet

3 participants