Skip to content

feat: show product-id for each offer when listing offers#16

Open
toabctl wants to merge 1 commit intocanonical:mainfrom
toabctl:main-product-id-in-offer-list
Open

feat: show product-id for each offer when listing offers#16
toabctl wants to merge 1 commit intocanonical:mainfrom
toabctl:main-product-id-in-offer-list

Conversation

@toabctl
Copy link
Collaborator

@toabctl toabctl commented May 29, 2024

A product can have multiple offers but a offer can have only a single product. So listing the product-id for each offer when showing a list of available offers is useful to get the relationship.

@toabctl toabctl requested a review from JessicaJang May 29, 2024 13:09
A product can have multiple offers but a offer can have only a single
product. So listing the product-id for each offer when showing a list
of available offers is useful to get the relationship.
@toabctl toabctl force-pushed the main-product-id-in-offer-list branch from 4674ab8 to 9c807a3 Compare May 29, 2024 13:24
Copy link
Contributor

@JessicaJang JessicaJang left a comment

Choose a reason for hiding this comment

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

Please fix the linting

awsmp/cli.py:71: error: Invalid index type "str" for "str"; expected type "SupportsIndex | slice" [index]
Found 1 error in 1 file (checked 13 source files)
Error: Sequence aborted after failed subtask 'mypy'
lint: exit 1 (10.12 seconds) /home/runner/work/awsmp/awsmp> poetry run poe lint pid=1863
lint: FAIL ✖ in 15.15 seconds

change-wise looks good to me

t.field_names = ["entity-id", "name", "visibility", "last-changed"]
if entity_type == "AmiProduct":
t.field_names = ["entity-id", "name", "visibility", "last-changed"]
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

we should be explicit on which entity types this is (e.g. in case we add SaaS later or other marketplace types)

Copy link
Collaborator Author

@toabctl toabctl Jun 3, 2024

Choose a reason for hiding this comment

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

you have to give the entity type at the command line. so imo you can't get this wrong:

awsmp inspect entity-list AmiProduct

vs.

awsmp inspect entity-list Offer

Copy link
Contributor

Choose a reason for hiding this comment

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

what about direct library users? if we add more supported entities?

t.add_row([entity["EntityId"], entity["Name"], entity["Visibility"], entity["LastModifiedDate"]])
if entity_type == "AmiProduct":
t.add_row([entity["EntityId"], entity["Name"], entity["Visibility"], entity["LastModifiedDate"]])
else:
Copy link
Contributor

Choose a reason for hiding this comment

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

again, explicit on entity_type

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