api: u.pro.services.list.v1 #3262

dheyay · 2024-08-12T19:57:29Z

Why is this needed?

Adds the following api endpoint:
u.pro.services.list.v1

Test Steps

(un)check this to re-run the checklist action

github-actions · 2024-08-12T19:57:42Z

PR Checklist

How to use this checklist

PR Author

For each section, check a box when it is true.
Uncheck a box if it becomes un-true.
Then check the box at the bottom of the PR description to re-run the action that creates this checklist.
The action that creates and updates this comment will retain your edits.
The action will fail if the checklist is not completed.

PR Reviewer

Check that the PR checklist action did not fail.
Double check that the author filled out the checklist accurately.
If you disagree with a checklist item, start a conversation.
For example, the author may say they don't think integration tests are necessary, but you may disagree.

Bug References

None.

Confirm

I've properly referenced all bugs that this PR fixes

How to properly reference fixed bugs

If this PR is related to a Jira item, include an SC-1234 reference in the PR title
If this PR is fixes a GitHub issue, include a Fixes: #1234 reference in the commit that fixes the issue
If this PR is fixes a Launchpad bug, include a LP: #12345678 reference in the commit that fixes the issue

Test Updates

Unit Tests

I have updated or added any unit tests accordingly
No unit test changes are necessary for this change

Integration Tests

I have updated or added any integration tests accordingly
No integration test changes are necessary for this change

Documentation

Changes here need to be documented and I have referenced the docs PR in the description
No documentation updates are necessary for this change

Does this PR require review from someone outside the core ubuntu-pro-client team?

Yes, and I have requested those reviews via GitHub
No

features/api_services_list.feature

lucasmoura · 2024-08-14T02:47:59Z

uaclient/api/u/pro/services/list/v1.py

+    return ServiceListResult(services=services_list)
+
+
+def _get_service_status(cfg, ent, inapplicable_resources):


I know this problem was not introduced in this PR, but I think an entitlement should be able to tell its status. We have logic on different places that check if a service is enabled. I think we should consolidate then on the entitlement class. What do you think @orndorffgrant @renanrodrigo ?

agreed, but in a separate changeset imho

lucasmoura

@dheyay Let's also add a description to the commit explaining what the API does

features/api_services_list.feature

dheyay · 2024-08-20T23:46:06Z

Updated @lucasmoura

features/api_services_list.feature

uaclient/api/u/pro/services/list/v1.py

renanrodrigo · 2024-08-21T12:03:17Z

No unit test changes are necessary for this change

Well you are technically adding a unit. Why wouldn't we need the test?

dheyay · 2024-08-22T00:22:07Z

Updated @renanrodrigo @lucasmoura

features/api_services_list.feature

uaclient/api/u/pro/services/list/v1.py

orndorffgrant · 2024-08-28T20:28:24Z

uaclient/api/u/pro/services/list/v1.py

+        entitled = False
+    elif contract_status == ContractStatus.ENTITLED:
+        entitled = True
+    available = True if ent.name not in inapplicable_resources else False


I think we should generally be using ent.applicability_status() for the value of available - right @lucasmoura @renanrodrigo ?

I think @dheyay is just replicating the logic we have on status here. However, I think it is worth testing out replacing this logic with applicability_status

It works with the applicability_status logic and does not need the additional inapplicable_resources. Updated the implementation and the endpoint/function names @orndorffgrant

renanrodrigo

So I still think we could have a couple unit tests for the basic functionality here. The API implementation file has 0% coverage
I'm fine if we merge this without the test for now though, as we have precedents - we already have API files at 0%.

uaclient/api/u/pro/services/list/v1.py

dheyay · 2024-08-30T22:23:45Z

Updated with a unit test @orndorffgrant @renanrodrigo

orndorffgrant

One minor update needed - preemptively approving

features/api/api.feature

Adds an api endpoint to return the list of services with their availability, entitlement and description

uses the newly added jq filter and schema to test the api output

lucasmoura reviewed Aug 14, 2024

View reviewed changes

dheyay force-pushed the status-api-serviceList branch from bc09e9d to 7928f80 Compare August 19, 2024 16:25

lucasmoura reviewed Aug 20, 2024

View reviewed changes

features/api_services_list.feature Outdated Show resolved Hide resolved

features/api_services_list.feature Outdated Show resolved Hide resolved

dheyay force-pushed the status-api-serviceList branch from 7928f80 to 001d6f5 Compare August 20, 2024 23:45

renanrodrigo reviewed Aug 21, 2024

View reviewed changes

dheyay force-pushed the status-api-serviceList branch from 001d6f5 to bec62b8 Compare August 22, 2024 00:20

lucasmoura reviewed Aug 22, 2024

View reviewed changes

features/api_services_list.feature Outdated Show resolved Hide resolved

features/api_services_list.feature Outdated Show resolved Hide resolved

dheyay force-pushed the status-api-serviceList branch 2 times, most recently from 606cd8b to 8978583 Compare August 22, 2024 20:11

lucasmoura approved these changes Aug 23, 2024

View reviewed changes

uaclient/api/u/pro/services/list/v1.py Show resolved Hide resolved

dheyay force-pushed the status-api-serviceList branch from 8978583 to 0b70283 Compare August 23, 2024 18:51

orndorffgrant reviewed Aug 28, 2024

View reviewed changes

dheyay force-pushed the status-api-serviceList branch 3 times, most recently from 39ca3de to 73c5558 Compare August 29, 2024 21:23

renanrodrigo approved these changes Aug 30, 2024

View reviewed changes

orndorffgrant reviewed Aug 30, 2024

View reviewed changes

uaclient/api/u/pro/services/list/v1.py Outdated Show resolved Hide resolved

orndorffgrant reviewed Aug 30, 2024

View reviewed changes

uaclient/api/u/pro/services/list/v1.py Outdated Show resolved Hide resolved

dheyay force-pushed the status-api-serviceList branch from 73c5558 to fb8d457 Compare August 30, 2024 21:02

orndorffgrant approved these changes Sep 4, 2024

View reviewed changes

features/api/api.feature Outdated Show resolved Hide resolved

dheyay added 4 commits September 4, 2024 09:46

api: u.pro.services.list.v1

fa39ef8

Adds an api endpoint to return the list of services with their availability, entitlement and description

Add new output check step

d7c3dbc

Add schema for the service list api response

2cca3c7

tests: Update integration test

86dee90

uses the newly added jq filter and schema to test the api output

dheyay force-pushed the status-api-serviceList branch from fb8d457 to 86dee90 Compare September 4, 2024 16:46

orndorffgrant merged commit 75129ed into next-v35 Sep 4, 2024
21 of 24 checks passed

orndorffgrant deleted the status-api-serviceList branch September 4, 2024 17:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: u.pro.services.list.v1 #3262

api: u.pro.services.list.v1 #3262

dheyay commented Aug 12, 2024

github-actions bot commented Aug 12, 2024 •

edited by dheyay

Loading

How to use this checklist

PR Author

PR Reviewer

lucasmoura Aug 14, 2024

renanrodrigo Aug 14, 2024

lucasmoura left a comment

dheyay commented Aug 20, 2024

renanrodrigo commented Aug 21, 2024

dheyay commented Aug 22, 2024

orndorffgrant Aug 28, 2024

lucasmoura Aug 29, 2024

dheyay Aug 29, 2024

renanrodrigo left a comment

dheyay commented Aug 30, 2024

orndorffgrant left a comment

		return ServiceListResult(services=services_list)


		def _get_service_status(cfg, ent, inapplicable_resources):

api: u.pro.services.list.v1 #3262

api: u.pro.services.list.v1 #3262

Conversation

dheyay commented Aug 12, 2024

Why is this needed?

Test Steps

github-actions bot commented Aug 12, 2024 • edited by dheyay Loading

PR Checklist

How to use this checklist

PR Author

PR Reviewer

Bug References

Confirm

Test Updates

Unit Tests

Integration Tests

Documentation

Does this PR require review from someone outside the core ubuntu-pro-client team?

lucasmoura Aug 14, 2024

Choose a reason for hiding this comment

renanrodrigo Aug 14, 2024

Choose a reason for hiding this comment

lucasmoura left a comment

Choose a reason for hiding this comment

dheyay commented Aug 20, 2024

renanrodrigo commented Aug 21, 2024

dheyay commented Aug 22, 2024

orndorffgrant Aug 28, 2024

Choose a reason for hiding this comment

lucasmoura Aug 29, 2024

Choose a reason for hiding this comment

dheyay Aug 29, 2024

Choose a reason for hiding this comment

renanrodrigo left a comment

Choose a reason for hiding this comment

dheyay commented Aug 30, 2024

orndorffgrant left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 12, 2024 •

edited by dheyay

Loading