Skip to content
This repository was archived by the owner on May 28, 2023. It is now read-only.

Corrige mimetype do get datasets #46

Merged
merged 1 commit into from
Sep 9, 2019
Merged

Corrige mimetype do get datasets #46

merged 1 commit into from
Sep 9, 2019

Conversation

JoseRenan
Copy link
Member

No PR #45 acabou passando despercebido que o GET /datasets estava retornando um JSON mas com o mimetype plain/text. Eu havia sugerido a edição do retorno do get_datasets pra usar o schema marshmallow pra gerar o JSON, ao invés do jsonify do flask, porém eu havia esquecido que o jsonify adiciona automaticamente o mimetype como application/json e o schema não... daí pra resolver isso eu podia escolher entre, adicionar o mimetype na mão, instalar a extensão flask-marshmallow que já tem um jsonify embutido no dump, ou simplesmente usar o jsonify em tudo... Escolhi a terceira opção pois é a mais simples e não precisa adicionar nenhuma nova dependencia.

Sugiro também que todos os outros endpoints usem como retorno o jsonify pra padronizarmos (Esse PR também resolve isso, afinal só temos dois endpoints msm kkkkkkkkk)

Copy link
Contributor

@paulojbleitao paulojbleitao left a comment

Choose a reason for hiding this comment

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

Achei top, mas será que não seria interessante fazer um decorator ou algo assim pra isso? Acho menos provável de passar batido (em vez de sempre botar o jsonify no retorno)

@JoseRenan
Copy link
Member Author

@paulojbleitao ter o decorator é top, mas não sei, acho que complicaria pra contribuidores entenderem já que o jsonify eles vão encontrar direto na documentação do flask e nos foruns quando pesquisar "como retornar json no flask", nusei 🤔

@paulojbleitao
Copy link
Contributor

@JoseRenan De fato 🤔 a gente pode deixar assim e se for um problema a gente retoma a ideia do decorator, então

@JoseRenan JoseRenan merged commit 78a780d into development Sep 9, 2019
@lucasmedeiros lucasmedeiros deleted the fix-jsonify branch September 9, 2019 21:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants