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

[0] Removendo campo `errors` do BaseModel #135

Merged
merged 51 commits into from Jul 23, 2019

Conversation

@daltonmatos
Copy link
Member

daltonmatos commented Jul 10, 2019

Esse campo foi uma tentativa de conseguir preencher eventuais erros no
momento de popular um modelo (Resource) qualquer.

Surgiu para sinalizar um problema no momento de preencher os campos
application e total_apps do modelo Agent.

Mas o problema é que como esse campo está no BaseModel, todos os
sub-modelos renderizam esse campo. Removi para podemos caminhar com
modelos mais limpos e depois precisamos pensar em uma forma de
sinzalizar esses erros eventuais, que são erros que não comprometem todo
o response de um request.

Por enquanto troquei o campo pro um logger.exception().

Esse campo foi uma tentativa de conseguir preencher eventuais erros no
momento de popular um modelo (Resource) qualquer.

Surgiu para sinalizar um problema no momento de preencher os campos
`application` e `total_apps` do modelo Agent.

Mas o problema é que como esse campo está no BaseModel, *todos* os
sub-modelos renderizam esse campo. Removi para podemos caminhar com
modelos mais limpos e depois precisamos pensar em uma forma de
sinzalizar esses erros eventuais, que são erros que não comprometem todo
o response de um request.

Por enquanto troquei o campo pro um `logger.exception()`.
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jul 10, 2019

Codecov Report

Merging #135 into master will increase coverage by 1.25%.
The diff coverage is 99.06%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #135      +/-   ##
==========================================
+ Coverage    90.2%   91.45%   +1.25%     
==========================================
  Files          86      101      +15     
  Lines        2592     3008     +416     
  Branches      156      166      +10     
==========================================
+ Hits         2338     2751     +413     
- Misses        241      244       +3     
  Partials       13       13
Flag Coverage Δ
#typehint 44.15% <89.18%> (+14.1%) ⬆️
#unittest 95.66% <98.24%> (+0.39%) ⬆️
Impacted Files Coverage Δ
asgard/handlers/http.py 0% <0%> (ø) ⬆️
asgard/api/resources/jobs.py 100% <100%> (ø)
asgard/models/account.py 68.18% <100%> (ø) ⬆️
asgard/clients/chronos/__init__.py 100% <100%> (ø)
asgard/backends/mesos/impl.py 100% <100%> (ø) ⬆️
asgard/conf.py 100% <100%> (ø) ⬆️
asgard/clients/chronos/models/job.py 100% <100%> (ø)
asgard/http/client.py 68.42% <100%> (+0.85%) ⬆️
asgard/models/spec/env.py 100% <100%> (ø)
asgard/models/spec/schedule.py 100% <100%> (ø)
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d98bf55...9570129. Read the comment docs.

daltonmatos added 23 commits Jul 10, 2019
Dessa forma continuamos podendo indicar ao Front que o carregamento
da lista de applications deu erro.
Dessa forma o codecov vai marcar como falha caso o coverage de unittest
tenha abaixado em qualquer quantidade.
Ainda não sei onde ficarão os modelos que são comuns a vários outros,
como por exemplo, ConstraintSpec, EnvSpec, etc.

Se formos chamá-los de `*Spec` talvez faça sentido estar em
`asgard.models.specs.*`.
Esses sub-modelos estão em `asgard.models.spec.*`
fmt
Quando usamos o client inteito no context block, assim:

```
from asgard.http.client import http_client

async with http_client as client:
  await client.get(...)
  await client.post(...)
```

A session deve permanecer aberta para que possamos ter novos blocos como
esse pelo código. Fechando a session on final do bloco ia nos obrigar a
criar uma nova session a cada novo bloco, o que nem é recomendado pela
doc do aiohttp client: https://docs.aiohttp.org/en/stable/client_quickstart.html

para os casos onde fazemos:

```
async with http_client.get(...) as response:
  ...
```

Estamos ainda criando uma nova session para cada context block.
O Ambiente local agora conta com um chronos rodando. Ele vai servir para
os testes de integração da api de scheduled jobs
Essa interface deve ser implementada por todos os backends que precisem
converter os models de seus clients internos (`asgard.clients.*.models.*`)
para os models do asgard (`asgard.models.*`).
Conversores de models do Chronos pra modelos do Asgard
fmt
@daltonmatos daltonmatos changed the title Removendo campo `errors` do BaseModel [0] Removendo campo `errors` do BaseModel Jul 18, 2019
daltonmatos added 4 commits Jul 18, 2019
As chaves são name e value e não `key` e `value`
Estava chamendo com os parametros invertidos
daltonmatos added 11 commits Jul 19, 2019
Se não fechamos a sessão, caso o loop seja trocado, o http_client
que já foi usado antes (e por isso já tem uma session aberta) para
de funcionar com o erro "loop is closed".

O que temos que fazer é melhorar a API desse http_client. Idealmente
exigino que ele seja instanciado antes de ser usado.
Em vez de fazermos assim:

```python
from asgard.http.client import http_client

async with http_client as client:
  await client.get(...)
```

Fazer algo nessa linha:
```python
from asgard.http.client import HttpClient

client = HttpClient(headers=..., ...)
await client.get(...)
```
namespace

Essa interface é quem vai implementar a adição/remoção do namespace
no nome das aplicações.
O início do teste faz um registro de um job novo no chronos,
mas log em seguida, quando buscamos esse job o chronos responde que ele
não existe.

Essa espera é para dar tempo do chronos registrar tudo e responder HTTP 200.
@pabrrs
pabrrs approved these changes Jul 22, 2019
daltonmatos added 12 commits Jul 22, 2019
Explica a nova arquitetura de models e ModelConverters.
Atualiza pontos onde o Chronos pode ser mencionado.
…uled-jobs-get-job-by-id
…duled-jobs-from-asgard-to-client-model
…o feature/scheduled-jobs-from-client-to-asgard-model
…o feature/scheduled-jobs-models
…s-key-from-models
@daltonmatos daltonmatos merged commit 9570129 into master Jul 23, 2019
7 checks passed
7 checks passed
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/typehint 89.18% of diff hit (target 30.05%)
Details
codecov/patch/unittest 98.24% of diff hit (target 95.26%)
Details
codecov/project/typehint 44.15% (+14.1%) compared to d98bf55
Details
codecov/project/unittest 95.66% (+0.39%) compared to d98bf55
Details
@daltonmatos daltonmatos deleted the feature/remove-erros-key-from-models branch Jul 23, 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.

None yet

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