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

✨ introduce licensed_items_purchases endpoints 🗃️ #6928

Merged
merged 31 commits into from
Dec 11, 2024
Merged

✨ introduce licensed_items_purchases endpoints 🗃️ #6928

merged 31 commits into from
Dec 11, 2024

Conversation

matusdrobuliak66
Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 commented Dec 9, 2024 •
edited
Loading

What do these changes do?

  • ✨ introduce licensed_items_purchases endpoints
  • ✨ implement purchase functionality
    image

Next step:

Related issue/s

How to test

  • webserver
    test_licensed_items_purchases_handlers.py
  • resource usage tracker
    test_api_licensed_items_purchases.py

Dev-ops checklist

@matusdrobuliak66 matusdrobuliak66 self-assigned this Dec 9, 2024
@matusdrobuliak66 matusdrobuliak66 added this to the Event Horizon milestone Dec 9, 2024
@matusdrobuliak66 matusdrobuliak66 added a:resource-usage-tracker resource usage tracker service a:webserver issue related to the webserver service labels Dec 9, 2024
@codecov Codecov
Copy link

codecov bot commented Dec 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 87.78626% with 48 lines in your changes missing coverage. Please review.

Project coverage is 87.91%. Comparing base (a35576c) to head (356b3ee).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #6928      +/-   ##
==========================================
+ Coverage   87.78%   87.91%   +0.13%     
==========================================
  Files        1586     1532      -54     
  Lines       62432    60721    -1711     
  Branches     2005     1753     -252     
==========================================
- Hits        54806    53384    -1422     
+ Misses       7290     7028     -262     
+ Partials      336      309      -27
Flag Coverage Δ
integrationtests 64.94% <73.75%> (-4.75%) ⬇️
unittests 86.08% <87.78%> (-0.18%) ⬇️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 91.24% <100.00%> (+0.16%) ⬆️
pkg_notifications_library 84.57% <ø> (ø)
pkg_postgres_database 88.07% <100.00%> (+0.01%) ⬆️
pkg_service_integration 70.02% <ø> (ø)
pkg_service_library 75.11% <0.00%> (-0.43%) ⬇️
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.38% <ø> (ø)
agent 97.00% <ø> (ø)
api_server 90.04% <ø> (ø)
autoscaling 95.21% <ø> (ø)
catalog 90.57% <ø> (ø)
clusters_keeper 99.48% <ø> (ø)
dask_sidecar 91.26% <ø> (ø)
datcore_adapter 93.18% <ø> (ø)
director 76.49% <ø> (ø)
director_v2 91.37% <ø> (-0.02%) ⬇️
dynamic_scheduler 96.99% <ø> (ø)
dynamic_sidecar 89.75% <ø> (ø)
efs_guardian 90.12% <ø> (ø)
invitations 93.44% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.65% <ø> (ø)
resource_usage_tracker 89.58% <93.18%> (+0.33%) ⬆️
storage 89.60% <ø> (ø)
webclient ∅ <ø> (∅)
webserver 87.44% <94.32%> (+0.85%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a35576c...356b3ee. Read the comment docs.

@matusdrobuliak66 matusdrobuliak66 changed the title WIP: db layer licensed items purchase ✨ introduce licensed_items_purchases endpoints Dec 10, 2024
@matusdrobuliak66 matusdrobuliak66 marked this pull request as ready for review December 10, 2024 14:19
odeimaiz
odeimaiz reviewed Dec 10, 2024
View reviewed changes
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Yeeeha!

sanderegg
sanderegg approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

great! thanks

packages/models-library/src/models_library/resource_tracker_licensed_items_purchases.py Show resolved Hide resolved
...e_postgres_database/migration/versions/8fa15c4c3977_add_cols_to_licensed_items_purchases_.py Show resolved Hide resolved
@matusdrobuliak66 matusdrobuliak66 changed the title ✨ introduce licensed_items_purchases endpoints ✨ introduce licensed_items_purchases endpoints 🗃️ Dec 10, 2024
pcrespov
pcrespov reviewed Dec 10, 2024
View reviewed changes
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

What do you want to represent with licensed_items_purchases: the transaction or the actual license?

You are introducing licensed_items_purchases as the resource obtained when purchasing a licensed_item. If you do not mean the transaction, isn't that essentially just a license?

  • The items that are licensed are called licensed_items.
  • To use a licensed_item, you must purchase the corresponding license.
  • In order to use a licensed_item, the system verifies the existence of the "linked" or "associated" license purchased for that specific purpose.

A selection of other names that chatty suggested in case you need it

If it represents the license:

  • usage_license
  • active_license

If it represents the purchase transaction:

  • license_purchase
  • purchase_record

If it represents both the purchase and the license:

  • licensed_item_access
  • purchased_license

The key is to align the name with the role this resource plays in the system. If it’s being used to check permissions (whether an item can be used), a term like license or licensed_item_license may work best. If it’s primarily about the transaction or tracking the purchase, then a term emphasizing "purchase" is more appropriate.

api/specs/web-server/_licensed_items_purchases.py Show resolved Hide resolved
api/specs/web-server/_licensed_items_purchases.py Outdated Show resolved Hide resolved
...ls-library/src/models_library/api_schemas_resource_usage_tracker/licensed_items_purchases.py Show resolved Hide resolved
...ls-library/src/models_library/api_schemas_resource_usage_tracker/licensed_items_purchases.py Show resolved Hide resolved
@matusdrobuliak66
Copy link
Contributor Author

What do you want to represent with licensed_items_purchases: the transaction or the actual license?

You are introducing licensed_items_purchases as the resource obtained when purchasing a licensed_item. If you do not mean the transaction, isn't that essentially just a license?

  • The items that are licensed are called licensed_items.
  • To use a licensed_item, you must purchase the corresponding license.
  • In order to use a licensed_item, the system verifies the existence of the "linked" or "associated" license purchased for that specific purpose.

A selection of other names that chatty suggested in case you need it

If it represents the license:

  • usage_license
  • active_license

If it represents the purchase transaction:

  • license_purchase
  • purchase_record

If it represents both the purchase and the license:

  • licensed_item_access
  • purchased_license

The key is to align the name with the role this resource plays in the system. If it’s being used to check permissions (whether an item can be used), a term like license or licensed_item_license may work best. If it’s primarily about the transaction or tracking the purchase, then a term emphasizing "purchase" is more appropriate.

In my naming, we are offering licensed-items, and when you purchase an item, a licensed-item-purchase is created (a "transaction" to reference your terminology). Unless someone has a strong opinion, I would keep it this way. I am in turbo mode to merge this and the upcoming PRs (already ongoing), as OM and MaG are waiting for them. They need to do some work on their side, and the deadline is on Monday.

bisgaard-itis
bisgaard-itis approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Very nice. Thank!

api/specs/web-server/_licensed_items_purchases.py Show resolved Hide resolved
...sage-tracker/src/simcore_service_resource_usage_tracker/services/licensed_items_purchases.py Show resolved Hide resolved
GitHK
GitHK approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

Just have a few questions below.

api/specs/web-server/_licensed_items_purchases.py Show resolved Hide resolved
...s-database/src/simcore_postgres_database/models/resource_tracker_licensed_items_purchases.py Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Dec 11, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@matusdrobuliak66 matusdrobuliak66 enabled auto-merge (squash) December 11, 2024 09:24
pcrespov
pcrespov approved these changes Dec 11, 2024
View reviewed changes
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

pair reviewed. thx

@pcrespov pcrespov disabled auto-merge December 11, 2024 10:18
@pcrespov pcrespov merged commit 825c22a into ITISFoundation:master Dec 11, 2024
90 of 93 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Dec 11, 2024
Closed
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@odeimaiz odeimaiz odeimaiz left review comments

@bisgaard-itis bisgaard-itis bisgaard-itis approved these changes

@sanderegg sanderegg sanderegg approved these changes

@pcrespov pcrespov pcrespov approved these changes

@GitHK GitHK GitHK approved these changes

@giancarloromeo giancarloromeo Awaiting requested review from giancarloromeo giancarloromeo is a code owner

Assignees

@matusdrobuliak66 matusdrobuliak66

Labels
a:resource-usage-tracker resource usage tracker service a:webserver issue related to the webserver service
Projects
None yet
Milestone
Event Horizon
Development

Successfully merging this pull request may close these issues.
6 participants
@matusdrobuliak66 @GitHK @pcrespov @odeimaiz @sanderegg @bisgaard-itis