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

[14.0][FIX] l10n_br_fiscal: Criado parâmetro para configurar o valor do tempo limite do serviço ibpt #3115

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

MateusONunes
Copy link
Contributor

Criado o parâmetro ibpt_request_timeout para configurar o limite do serviço de ibpt. O Parâmetro ibpt_request_timeout poderá ser adicionado nas configurações de parâmetros do Odoo(ir.config_parameter) ou no arquivo odoo.conf. Esta PR é um complemento da PR#3111

@OCA-git-bot
Copy link
Contributor

Hi @renatonlima,
some modules you are maintaining are being modified, check this out!

Copy link
Member

@renatonlima renatonlima left a comment

Choose a reason for hiding this comment

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

Olá @MateusONunes,

Obrigado pela contribuição, ao invés de alterar as funções get_ibpt_product, service no arquivo l10n_br_fiscal/models/ibpt.py para passar o env, e acabou gerando algumas alterações como nos testes para adaptar a assinatura da função.

Ao invés de passar o env para os métodos, você poderia alterar o método action_ibpt_inquiry na classe DataNcmNbsAbstract, se você objsevar na linha 89 você vai ver que é criado uma instancia do objeto DeOlhoNoImposto para passar o token e cnpj e uf, você poderia injetar a informação de timeout, dessa forma você iria conseguir usar esse parâmetro na consulta ao webservice sem ter que fazer muitas alterações

@MateusONunes
Copy link
Contributor Author

Obrigado pelas orientações @renatonlima. Coloquei o env no objeto DeOlhoNoImposto e ficou mais enxuto o código. Fico no aguardo caso precise de mais correções, obrigado.

@@ -90,6 +90,7 @@ def action_ibpt_inquiry(self):
company.ibpt_token,
misc.punctuation_rm(company.cnpj_cpf),
company.state_id.code,
self.env,
Copy link
Member

Choose a reason for hiding this comment

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

Eu acho que seria melhor ao invés de você passar o env aqui, passar o request_timeout



def _request(ws_url, params):
def _request(ws_url, params, env=None):
Copy link
Member

Choose a reason for hiding this comment

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

Seria melhor ter o argumento request_timeout=30 ao invés de env

try:
response = requests.get(ws_url, params=params, timeout=30)
if env:
ibpt_request_timeout = (
Copy link
Member

Choose a reason for hiding this comment

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

Esse código poderia ficar antes do https://github.com/OCA/l10n-brazil/blob/14.0/l10n_br_fiscal/models/data_ncm_nbs_abstract.py#L89 e passar o valor do request_timeout dentro o config

Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

LGTM

@rvalyi
Copy link
Member

rvalyi commented Jun 13, 2024

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-3115-by-rvalyi-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 21d9cd5 into OCA:14.0 Jun 13, 2024
6 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at a122963. Thanks a lot for contributing to OCA. ❤️

@MateusONunes
Copy link
Contributor Author

Seria válido aplicar esta mesma correção na versão 15 e 16? Atualmente em ambas o timeout está 15 segundos. Para fazer isto teria que criar 2 PRs para cada versão ou existe algum procedimento padrão de migração do commit?

@marcelsavegnago
Copy link
Member

marcelsavegnago commented Jun 13, 2024

@MateusONunes sim.. é bom sim, faça o cherry-pick deste commit e submete uma PR para cada versão por favor.

@rvalyi
Copy link
Member

rvalyi commented Jun 13, 2024

alias @MateusONunes parabens pela primeira contribuiçao no projeto (acabei de me tocar agora que era seu primeiro commit).

@marcelsavegnago
Copy link
Member

Opa.. verdade.. parabens @MateusONunes e obrigado pela contribuição.

@MateusONunes
Copy link
Contributor Author

Ahhh, muito obrigado, felicidade imensa pessoal. Só mesmo agradecer a vocês pelo acolhimento, apoio, profissionalismo, seriedade, paciência, vontade, conhecimento de vocês e por tudo que estou aprendendo. Quero me envolver mais com este projeto, minha meta é ultrapassar os commits da OCA-git-boot kkkk. Fiquei com bastante receio de entrar, perguntar, criar a PR... mas tem uma mensagem antiga no grupo da OCA que o @marcelsavegnago fala que "tem que botar a cara a tapa" e confesso que isto ajudou bastante pra dar coragem, kkkk.

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

Successfully merging this pull request may close these issues.

5 participants