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

Implementa handlers `jobs/*` com assinatura dinâmica #165

Merged
merged 14 commits into from Oct 15, 2019

Conversation

@daltonmatos
Copy link
Member

daltonmatos commented Aug 7, 2019

Nota: Esse PR é apenas uma demonstração, pois usa o asyncowker direto da master. Mas assim que lançarmos a próxima versão do asyncorker poderemos migrar os outros endpoints e fazer o merge desse PR.

Agora que o asyncowrker possui uma infra-estrutura básica para resolução
de parametros de uma corotina, é possível declarar handlers HTTP que já
recebem objetos prontos e validados em vez de receber sempre
web.Request.

Esse PR demonstra como ficariam os handlers implementados usando essa
nova possibilidade.

Como exemplo, migrei os handlers de /jobs/*

Tarefas

  • A atualização dessa branch com o código do PR b2wdigital/async-worker#129 trouxe junto o pydantic==0.28.0. E isso significa que agora temos que resolver alguns erros de lint (#138). Vamos resolver esses erro aqui nessa branch mesmo (ou talvez abrir uma segunda) e depois fazemos o merge para a master.

Closes #138

async def index_jobs(request: web.Request):
user = await User.from_alchemy_obj(request["user"])
account = await Account.from_alchemy_obj(request["user"].current_account)
async def index_jobs(request: web.Request, user: User, account: Account):
job_id = request.match_info["job_id"]

This comment has been minimized.

Copy link
@daltonmatos

daltonmatos Aug 7, 2019

Author Member

Ainda não temos suporte no asyncowker para extrair parametros da URL, por isso o web.Request ainda é necessário aqui.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Aug 14, 2019

Codecov Report

Merging #165 into master will decrease coverage by 0.05%.
The diff coverage is 97.72%.

Flag Coverage Δ
#typehint 46.85% <36.11%> (+0.62%) ⬆️
#unittest 97.33% <100%> (-0.02%) ⬇️
Impacted Files Coverage Δ
asgard/backends/base.py 100% <ø> (ø) ⬆️
asgard/app.py 84.84% <100%> (+1.51%) ⬆️
asgard/backends/mesos/models/app.py 100% <100%> (ø) ⬆️
asgard/models/task.py 100% <100%> (ø) ⬆️
asgard/backends/mesos/impl.py 100% <100%> (ø) ⬆️
asgard/models/base.py 95% <100%> (ø) ⬆️
asgard/api/jobs.py 89.58% <100%> (-1.65%) ⬇️
asgard/models/app.py 100% <100%> (ø) ⬆️
asgard/backends/mesos/models/agent.py 77.35% <100%> (ø) ⬆️
asgard/workers/autoscaler/app.py 100% <100%> (ø) ⬆️
... and 8 more
@pabrrs
pabrrs approved these changes Aug 30, 2019
Copy link
Collaborator

pabrrs left a comment

Quero isso na minha vida !

👏 👏 🎉

@daltonmatos

This comment has been minimized.

Copy link
Member Author

daltonmatos commented Aug 30, 2019

Quero isso na minha vida !

❤️

daltonmatos added 2 commits Aug 7, 2019
Agora que o asyncowrker possui uma infra-estrutura básica para resolução
de parametros de uma corotina, é possível declarar handlers HTTP que já
recebem objetos prontos e validados em vez de receber sempre
`web.Request`.

Esse PR demonstra como ficariam os handlers implementados usando essa
nova possibilidade.

Como exemplo, migrei os handlers de `/jobs/*`
Esse código foi revertido no merge com a master
@daltonmatos daltonmatos force-pushed the feature/dynamic-handler-parameters branch from ef17be2 to 1aee23f Oct 10, 2019
Esse código já está pronto para rodar com a implementação da
unificação das Apps, todas derivando de `asyncworker.App`.
@daltonmatos daltonmatos force-pushed the feature/dynamic-handler-parameters branch from 1aee23f to e3df62e Oct 10, 2019
daltonmatos added 7 commits Oct 11, 2019
Por alguma razão, quando usamos `staticmethod` o mypy continua reclamando
que a assinatura do método é incompatível com a assinatura do super
type, mas usando `classmethod` funciona.

como a chamada do método é igual (`Class.method()`) não tem problema
fazermos essa mudança.
Como os dois objetos (UsedResourcesSpec e ResourcesSpec) são, por
enquanto, iguais tudo funcionava.
Essencialmente esse commit silencia o linter nesses casos. Já existe uma
intenção de mudar o código de forma que poderemos remover esses ignores.

Isso acontecerá quando removermos por completo a classe `MesosAgent`.
Dessa forma o os converters do backend `mesos` vão transformar direto
de `*.clients.mesos.models.agent.MesosAgent` para
`asgard.models.agent.Agent`.
…er-parameters
@daltonmatos daltonmatos marked this pull request as ready for review Oct 11, 2019
@daltonmatos daltonmatos requested review from pabrrs and george-veras Oct 11, 2019

req_body = await request.json()
asgard_job = ScheduledJob(**req_body)
async def create_job(job: ScheduledJob, user: User, account: Account):

This comment has been minimized.

Copy link
@diogommartins

diogommartins Oct 13, 2019

Contributor

👏👏👏👏👏👏👏👏👏👏👏

daltonmatos added 4 commits Oct 15, 2019
Se geramos com py3.7 ficam faltando dois pacotes:

 - dataclass
 - idna-ssl
@daltonmatos daltonmatos merged commit 5cda5f2 into master Oct 15, 2019
6 of 7 checks passed
6 of 7 checks passed
codecov/patch/typehint 36.11% of diff hit (target 46.23%)
Details
ci/circleci: py368 Your tests passed on CircleCI!
Details
ci/circleci: py36x Your tests passed on CircleCI!
Details
ci/circleci: py37x Your tests passed on CircleCI!
Details
codecov/patch/unittest 100% of diff hit (target 97.34%)
Details
codecov/project/typehint 46.85% (+0.62%) compared to fca0ba9
Details
codecov/project/unittest Absolute coverage decreased by -0.01% but relative coverage increased by +2.65% compared to fca0ba9
Details
@daltonmatos daltonmatos deleted the feature/dynamic-handler-parameters branch Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.