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

Implement retrieval "Farm" APIs #554

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

hoangpn
Copy link
Contributor

@hoangpn hoangpn commented Nov 18, 2024

Adding 2 APIs for retrieving Farm's data:

  1. GET /rest/farm
    Returns a JSON array of all farms in Promgen.
    Available request params:
  • source: string. The returned array of farms will be of this source type. Exact match.
  • name: string. The returned array of farms will have the name which contains the parameter's value. Contains

Sample response:

[
    {
        "id": 1,
        "url": "http://127.0.0.1:8000/farm/1",
        "name": "farm-test",
        "source": "promgen"
    },
    ...
]
  1. GET /rest/farm/:id
    Returns the farm matching the ID as a JSON object, including the list of hosts.

Sample response:

{
    "id": 1,
    "url": "http://127.0.0.1:8000/farm/1",
    "name": "farm-test",
    "source": "promgen",
    "hosts": [
        {
            "url": "http://127.0.0.1:8000/host/host-test.com",
            "name": "host-test.com"
        },
        ...
    ]
}

@hoangpn hoangpn requested review from kfdm and a team as code owners November 18, 2024 10:05
@CLAassistant
Copy link

CLAassistant commented Nov 18, 2024

CLA assistant check
All committers have signed the CLA.

@hoangpn hoangpn marked this pull request as draft November 18, 2024 10:05
@hoangpn hoangpn force-pushed the add-farm-rest-api branch 2 times, most recently from 87970e1 to b78c3fa Compare November 19, 2024 02:07
@hoangpn hoangpn marked this pull request as ready for review November 19, 2024 02:09
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

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

I have reviewed and tested your code.

I have left some comments for requested changes. Please check them.
If you have any question about them, please let me know.

Also, please add a commit message explaining the context for this commit. For instance, why are we doing all of this? Why do we need this new API end point?

Thank you very much!!!

promgen/filters.py Outdated Show resolved Hide resolved
promgen/filters.py Outdated Show resolved Hide resolved
promgen/rest.py Show resolved Hide resolved
promgen/rest.py Outdated Show resolved Hide resolved
promgen/tests/test_rest.py Outdated Show resolved Hide resolved
@hoangpn hoangpn force-pushed the add-farm-rest-api branch 2 times, most recently from f4d617c to 3e543e5 Compare November 20, 2024 02:07
@hoangpn
Copy link
Contributor Author

hoangpn commented Nov 20, 2024

@vincent-olivert-riera
Thank you for your comments. I have updated the source code according to your suggestion!

Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

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

Thank you for addressing all the comments. This looks good to merge, but there are a couple of missing new-line characters in the two JSON files that we have added. Please fix them.

Another thing I wanted to tell you. Since you have force-pushed all your changes, I had to review all your code again.
Instead, it is better to use the --fixup option. For instance, if you are making changes to the code that was previously added by commit 3e543e5, after doing git add to add the new changes, use git commit --fixup=3e543e5 to create a new commit, and then push it. That way you can tell me "I have addressed this comment on commit XXXXXX", and I can review only that.

After the review is complete and the code is considered good, you can easily squash all those extra "fixup" commits automatically by doing: git rebase -i master --autosquash, and do a final force-push.

Please, try to do it that way for the requested changes in the JSON files. If you need any help, please ask me.

Thank you very much!

promgen/tests/examples/rest.farm.1.json Outdated Show resolved Hide resolved
promgen/tests/examples/rest.farm.json Outdated Show resolved Hide resolved
Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Please, squash all the fixup commits for final review and approval.

Thanks!

We intend to improve the UI/UX when adding a new Project by enhancing the registration process for Farms on the "Register Project" page.
To prepare for this improvement, we have added two API endpoints to retrieve Farm's data.
@hoangpn
Copy link
Contributor Author

hoangpn commented Nov 20, 2024

Thank you so much. I squashed and force pushed for final review. 🙇

Copy link
Contributor

@vincent-olivert-riera vincent-olivert-riera left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@vincent-olivert-riera vincent-olivert-riera merged commit 7f3f578 into line:master Nov 20, 2024
5 checks passed
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.

3 participants