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

Feature/async prediction #541

Merged
merged 11 commits into from
Nov 11, 2021
Merged

Feature/async prediction #541

merged 11 commits into from
Nov 11, 2021

Conversation

fberanizo
Copy link
Member

Adds a new Service "PredictionApi" …
Performs prediction tasks (fetch, create, and interrupt).
Also removes prediction actions that were at DeploymentsApi.js

Adds a new store 'prediction' to handle prediction actions …
These actions are similar to actions at 'testDeployment' store, but
predictions are now async.

Removes unused store 'testDeployment' …
This store was replaced by the new store 'prediction', which performs
similar actions, but asynchronously.

Updates containers to use actions of new store 'prediction' …

  • Removes occurrences of 'testDeployment' store
  • Adds a polling that checks for prediction results.

Renames remaining occurrences of 'testDeployment' to 'createPrediction' …
Ensures same descriptions are used across the project.

Performs prediction tasks (fetch, create, and interrupt).
Also removes prediction actions that were at DeploymentsApi.js
These actions are similar to actions at 'testDeployment' store, but
predictions are now async.
This store was replaced by the new store 'prediction', which performs
similar actions, but asynchronously.
- Removes occurrences of 'testDeployment' store
- Adds a polling that checks for prediction results.
Ensures same descriptions are used across the project.
@github-actions
Copy link

Dicas para revisão de código

Commits

  • Título (1a linha do commit): apresentar resumo do que foi alterado/adicionado/removido.
    ex: adiciona action que salva parametros no backend; exibe rótulo no componente de selecao de dataset;
  • Descrição (outras linhas): dar mais detalhes de cada alteração:
    • motivos das alterações
      ex: havia um bug que causava...; nova funcionalidade que faz isso...; código foi movido para...;
    • bibliotecas adicionadas e versões (package.json)
      ex: atualiza para antd v4.6;
    • testes unitários criados/alterados
      ex: adiciona testes para a action fetchProjectSuccess;
  • Mensagens auto-explicativas! Quem revisa o código deve entender o que foi feito (e porque foi feito) sem perguntar para quem fez o commit.
  • Não devem ter conflitos. Solicitar que sejam resolvidas as ocorrências de "This branch has conflicts that must be resolved".

SonarCloud Quality Gate

  • Coverage > 80.0%, e sempre que possível = 100%
  • 0 Bugs, 0 Code Smells, 0 Vulnerabilities

Build Github actions COM SUCESSO

ReactJS

  • Usar Node 12 ou 14.
  • Remover console.log.
  • Não deixar código-fonte comentado.
  • Pasta de módulos devem ser escritas em lower case
  • Pasta de componentes devem ser escritas em camel case
  • Nunca utilizar spread em retorno de reducer sem alteração
  • Sempre utilizar hooks ao invés de HOCs
  • Itens em uma lista devem ser renderizados com uma chave única (preferir uuid)
  • Comentar apenas códigos que não estão muito claros
  • Não utilizar comentários para blocos
  • Action types devem conter @

Redux

As instruções a seguir foram retiradas do Redux Style Guide.

@fberanizo
Copy link
Member Author

Não entendi porque o sonar identificou status e 'started' como tipos diferentes :x
image

@LuanEdCosta
Copy link
Contributor

@fberanizo Também não entendi aquele erro do sonar, mas pode ser por causa do != antes dele. Em JS geralmente usamos === ou na negação !==.

image
https://stackoverflow.com/questions/42517721/difference-between-and

It's not the final solution tough. Modal still blocks the access to
the 'Interrupt' button.
Results should be displayed in a different interface, maybe the logs/message section.
Removes action 'createPredictionWithFile' (it was replaced by 'createPredictionWithDataset').
Removes 'dataset' from payload of 'CREATE_PREDICTION_WITH_DATASET_REQUEST' actionType.
Remover selector 'getFile'
Tests, actions, actionTypes, reducer and selectors.
…redictionRequest

FETCH_PREDICTION_FAIL: don't set values in the state
fetchPredictionRequest: when prediction is still in progress, don't set status/predictionResult.
…is closed)

We have to do this because the user may have closed the modal, and there's no other way to reopen it.
@fberanizo fberanizo requested a review from LuanEdCosta November 6, 2021 15:56
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

68.4% 68.4% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@LuanEdCosta LuanEdCosta left a comment

Choose a reason for hiding this comment

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

Para mim está OK, mas realmente o modal não parece ser a melhor forma de fazer a funcionalidade de prediction.


Tive algumas ideias para o testar fluxo:

  • Podemos criar uma visualização abaixo ou acima de monitoramentos
  • Podemos fazer um outro botão de dropdown ao lado do botão de testar fluxo 🔽
  • Podemos mostrar em um dropdown de notificações.

image

@fberanizo
Copy link
Member Author

Para mim está OK, mas realmente o modal não parece ser a melhor forma de fazer a funcionalidade de prediction.

Tive algumas ideias para o testar fluxo:

  • Podemos criar uma visualização abaixo ou acima de monitoramentos
  • Podemos fazer um outro botão de dropdown ao lado do botão de testar fluxo 🔽
  • Podemos mostrar em um dropdown de notificações.

image

@LuanEdCosta acho que faremos com o Dropdown de notificações, sim!
No protótipo tem algumas coisas:
image

Vou dar merge neste PR e criar uma tarefa pra implementar essas melhorias.

@fberanizo fberanizo merged commit 44490d8 into master Nov 11, 2021
@LuanEdCosta LuanEdCosta deleted the feature/async-prediction branch February 22, 2022 13:16
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.

2 participants