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

feat: add tree-list endpoint #37

Merged
merged 4 commits into from
Jul 4, 2024
Merged

feat: add tree-list endpoint #37

merged 4 commits into from
Jul 4, 2024

Conversation

lfjnascimento
Copy link
Contributor

add tree-list endpoint serving a mock data for now

Description

Related Issues

How to test it

start docker services and make a get request to /tree/

docker compose up --build -d
curl http://localhost/tree/

you can also check the response in your browser at http://localhost/tree/

@lfjnascimento lfjnascimento requested a review from mari1912 June 28, 2024 20:34
@lfjnascimento lfjnascimento self-assigned this Jun 28, 2024
@lfjnascimento lfjnascimento requested a review from rfgvieira June 28, 2024 20:34
@lfjnascimento lfjnascimento marked this pull request as draft June 28, 2024 20:50
"branch": "linux-5.15",
"commit": "asidnasidn-oqiwejeoij-oaidnosdnk",
"buildStatus": "150 completed",
"testStatus": "80 completed"
Copy link
Member

Choose a reason for hiding this comment

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

likely this won't be a string like this, but rather an object with individual entries and the frontend will compute the string.

Either a summary object such as testStatus: { fail: 0, success: 80 }, or an array of each test and their status. For the listing, likely the summary object is better (less data).

@lfjnascimento lfjnascimento force-pushed the feat/add_tree_endpoint branch 5 times, most recently from 1578003 to b4e7ab1 Compare July 3, 2024 14:14
@lfjnascimento lfjnascimento marked this pull request as ready for review July 3, 2024 16:27
Comment on lines +10 to +14
'field_timestamp', 'id', 'origin', 'tree_name',
'git_repository_url', 'git_commit_hash', 'git_commit_name',
'git_repository_branch', 'patchset_files', 'patchset_hash',
'message_id', 'comment', 'start_time', 'contacts',
'log_url', 'log_excerpt', 'valid', 'misc'
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we only select the fields we are going to use for now? it will likely change in the future, but for now i don't see the need to get all of them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The REST API generally does not have this type of relationship with the front-end, it provides data about a resource but does not know what front-end applications will need or not.

return config.get(field)

def get_build_status(self, obj):
return self.get_field_from_config(obj, 'build_status')
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about extract those build_status, test_status, tree_name into variables? this way if there is some change in the database schema we only have to change it in one place

@lfjnascimento lfjnascimento force-pushed the feat/add_tree_endpoint branch from b4e7ab1 to b320a7c Compare July 4, 2024 13:11
for now we are using a hard-coded configuration, this is a
temporary solution while we don't have a table for it
@lfjnascimento lfjnascimento force-pushed the feat/add_tree_endpoint branch from b320a7c to d176e45 Compare July 4, 2024 13:22
@lfjnascimento lfjnascimento merged commit d176e45 into main Jul 4, 2024
2 checks passed
@lfjnascimento lfjnascimento deleted the feat/add_tree_endpoint branch July 4, 2024 13:23
@lfjnascimento lfjnascimento changed the title feat: add tree-list endpoint serving a mock data for now feat: add tree-list endpoint Jul 4, 2024
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.

Define endpoints required for Trees Monitor Listing Page and implement them
3 participants