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.

Sign up for GitHub

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

Jump to bottom

Start execution api #73

Merged
merged 16 commits into from
Nov 15, 2022
Merged

Start execution api #73

merged 16 commits into from
Nov 15, 2022

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Nov 11, 2022 •
edited
Loading

Description

This PR adds:

  • the ability to provide wanda the catalog path
  • an API to start a checks execution
  • a simple docker-compose setup for check development
  • basic doc to get started

For the documentation, to get a more visual review consider navigating here https://github.com/trento-project/wanda/tree/start_execution_api#readme

@nelsonkopliku nelsonkopliku self-assigned this Nov 11, 2022
@nelsonkopliku nelsonkopliku added enhancement New feature or request documentation Improvements or additions to documentation labels Nov 14, 2022
@nelsonkopliku nelsonkopliku marked this pull request as ready for review November 14, 2022 11:20
@nelsonkopliku nelsonkopliku force-pushed the start_execution_api branch 2 times, most recently from f123939 to 3d7af31 Compare November 14, 2022 11:23
fabriziosestito
fabriziosestito suggested changes Nov 14, 2022
View reviewed changes
Copy link
Member

@fabriziosestito fabriziosestito left a comment

Choose a reason for hiding this comment

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

Overall LGTM, Just a couple of nitpicks and a test missing

README.md Outdated Show resolved Hide resolved
lib/wanda/executions/target.ex Outdated Show resolved Hide resolved
test/wanda_web/controllers/execution_controller_test.exs Outdated Show resolved Hide resolved
arbulu89
arbulu89 requested changes Nov 14, 2022
View reviewed changes
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Hey @nelsonkopliku ,
Some comments

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
lib/wanda_web/controllers/execution_controller.ex Show resolved Hide resolved
lib/wanda_web/schemas/execution/start_execution_request.ex Outdated Show resolved Hide resolved
test/wanda_web/controllers/execution_controller_test.exs Outdated Show resolved Hide resolved
lib/wanda_web/controllers/execution_controller.ex Outdated Show resolved Hide resolved
test/wanda_web/controllers/execution_controller_test.exs Show resolved Hide resolved
arbulu89
arbulu89 reviewed Nov 14, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
@nelsonkopliku nelsonkopliku force-pushed the start_execution_api branch 2 times, most recently from 694c2e8 to ec8209a Compare November 14, 2022 17:23
arbulu89
arbulu89 approved these changes Nov 15, 2022
View reviewed changes
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Hey @nelsonkopliku ,
Thank you for the changes. It looks perfect now!
I have added some cosmetic related comments, feel free to ignore them if you want

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
lib/wanda/executions/target.ex Outdated Show resolved Hide resolved
lib/wanda_web/schemas/accepted_execution_response.ex Outdated Show resolved Hide resolved
test/executions/target_test.exs Outdated Show resolved Hide resolved
test/executions/target_test.exs Outdated Show resolved Hide resolved
test/wanda_web/controllers/execution_controller_test.exs Outdated Show resolved Hide resolved
@nelsonkopliku nelsonkopliku merged commit 28f7eb1 into main Nov 15, 2022
@nelsonkopliku nelsonkopliku deleted the start_execution_api branch November 15, 2022 10:41
@nelsonkopliku nelsonkopliku mentioned this pull request Nov 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arbulu89 arbulu89 arbulu89 approved these changes

@fabriziosestito fabriziosestito fabriziosestito approved these changes

@dottorblaster dottorblaster Awaiting requested review from dottorblaster

@rtorrero rtorrero Awaiting requested review from rtorrero

@abravosuse abravosuse Awaiting requested review from abravosuse

@stefanotorresi stefanotorresi Awaiting requested review from stefanotorresi

Assignees

@nelsonkopliku nelsonkopliku

Labels
documentation Improvements or additions to documentation enhancement New feature or request
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nelsonkopliku @fabriziosestito @arbulu89