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

Adiciona timeout em todos os provedores #190

Closed
wants to merge 2 commits into from
Closed

Conversation

filipedeschamps
Copy link
Member

Padrão de 2 segundos, menos o Correios que coloquei 5 segundos.

Motivo: nos logs do BrasilAPI estamos com algumas requests estourando o limite de 60 segundos de execução.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a16b0aa on timeout into 9bca1c4 on master.

@filipedeschamps
Copy link
Member Author

Será que tá muito curto o timeout? To pensando numa blackfriday, como vai se comportar os serviços.

@ivancorrea
Copy link

Será que tá muito curto o timeout? To pensando numa blackfriday, como vai se comportar os serviços.

@filipedeschamps

Acredito que os timeouts estão condizentes. Porém eu amentaria um pouco, visto que temos a chamada paralela em vários serviços, e aquele que for mais rápido, vence. Para o usuário, mesmo demorando um pouco, ele terá os dados retornados. Dando timeout em todas as respostas, o usuário não terá o dado, e precisará preencher o campos, caso seja um formulário.

O ideal mesmo, seria tornar isto uma configuração, onde o desenvolvedor possa configurar conforme a necessidade do projeto dele.

@LorhanSohaky
Copy link
Member

@ivancorrea , no PR #191 precisei modificar para que seja possível passar configurações aos services; se ele for aprovado, podem usar isso

@lucianopf
Copy link
Member

@ivancorrea , no PR #191 precisei modificar para que seja possível passar configurações aos services; se ele for aprovado, podem usar isso

Curto essa ideia tbm!

Mestre, eu acho 5 segundos ok tbm mas toma aí mais alguns insumos pra decisão 😬

Screen Shot 2020-10-07 at 08 09 23

Screen Shot 2020-10-07 at 08 08 05

Screen Shot 2020-10-07 at 08 07 52

Screen Shot 2020-10-07 at 08 07 43

Obs: Acredito que o BrasilAPI demora pra responder por alguma intermitência no Correios, repara que o tempo pico do Correios é super próximo ao do BrasilAPI 🤔 (poderia tbm ser algo na infra do serviço que usei pra monitorar 😬 )

@lucianopf
Copy link
Member

Mestres, só pra não deixar vcs perdidos abri uma issue com um planinho de ação pra reorganizar o repo dado a migração pra org do BrasilAPI 😬

#197

@lucianopf
Copy link
Member

Dado que esse PR está de certa forma sendo atacado aqui em #206 e permitindo mais customização irei fechar esse PR.

Fiquem a vontade de comentar no outro PR ou de reabrir esse PR! 😬 🙏 🤘

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.

5 participants