✨ Introduce wallet exchange 🗃️ #7033

matusdrobuliak66 · 2025-01-13T16:35:06Z

What do these changes do?

Currently, a project's connected wallet can go into a negative balance. This occurs because we have a policy to avoid interrupting users' computational jobs when their wallet reaches 0 credits—only dynamic services are stopped.

This PR:

🐛 Fixes a bug that prevents users from changing a project's wallet when the project is in DEBT.
- A project is considered IN DEBT if the wallet connected to it has a balance below 0 credits.
- Users can resolve this in two ways:
  - Top up credits to the wallet connected to the project.
  - Use a different wallet to pay off the project's debt.

Example:
The project transaction is in debt:

After the user pays off the debt using another wallet (triggering 2 new transactions between the wallets), the project transaction status changes from IN_DEBT to BILLED, unblocking the project.

Related issue/s

closes Issues when changing wallet after running out of credits osparc-issues#1791

How to test

Dev-ops checklist

No ENV changes or I properly updated ENV (read the instruction)

codecov · 2025-01-13T16:43:11Z

Codecov Report

Attention: Patch coverage is 79.79452% with 59 lines in your changes missing coverage. Please review.

Project coverage is 85.27%. Comparing base (3d01781) to head (61bbbf9).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7033      +/-   ##
==========================================
- Coverage   87.57%   85.27%   -2.30%     
==========================================
  Files        1629     1574      -55     
  Lines       63454    62079    -1375     
  Branches     2047     1794     -253     
==========================================
- Hits        55569    52940    -2629     
- Misses       7549     8828    +1279     
+ Partials      336      311      -25

Flag	Coverage Δ
integrationtests	`63.73% <43.03%> (-0.04%)`	⬇️
unittests	`83.62% <79.79%> (-2.35%)`	⬇️

Components	Coverage Δ
api	`76.84% <ø> (ø)`
pkg_aws_library	`∅ <ø> (∅)`
pkg_dask_task_models_library	`∅ <ø> (∅)`
pkg_models_library	`91.46% <100.00%> (+<0.01%)`	⬆️
pkg_notifications_library	`84.57% <ø> (ø)`
pkg_postgres_database	`88.31% <100.00%> (+0.01%)`	⬆️
pkg_service_integration	`70.02% <ø> (ø)`
pkg_service_library	`73.48% <0.00%> (-0.41%)`	⬇️
pkg_settings_library	`∅ <ø> (∅)`
pkg_simcore_sdk	`85.38% <ø> (ø)`
agent	`96.45% <ø> (ø)`
api_server	`90.54% <ø> (ø)`
autoscaling	`96.09% <ø> (ø)`
catalog	`90.66% <ø> (ø)`
clusters_keeper	`99.24% <ø> (ø)`
dask_sidecar	`91.26% <ø> (ø)`
datcore_adapter	`93.18% <ø> (ø)`
director	`76.42% <ø> (-0.09%)`	⬇️
director_v2	`91.30% <ø> (ø)`
dynamic_scheduler	`97.21% <ø> (ø)`
dynamic_sidecar	`89.75% <ø> (ø)`
efs_guardian	`90.30% <0.00%> (-0.17%)`	⬇️
invitations	`93.44% <ø> (ø)`
osparc_gateway_server	`∅ <ø> (∅)`
payments	`92.66% <ø> (ø)`
resource_usage_tracker	`89.15% <90.17%> (-0.31%)`	⬇️
storage	`89.57% <ø> (ø)`
webclient	`∅ <ø> (∅)`
webserver	`79.03% <87.34%> (-6.92%)`	⬇️

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 3d01781...61bbbf9. Read the comment docs.

….com:matusdrobuliak66/osparc-simcore into changing-wallet-after-running-out-of-credits

sonarqubecloud · 2025-01-18T14:09:36Z

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

odeimaiz

👌

sanderegg

All good, but the change in the efs-guardian. I would like to discuss this.

sanderegg · 2025-01-19T20:40:00Z

packages/models-library/src/models_library/resource_tracker.py

@@ -38,16 +38,50 @@ class ServiceRunStatus(StrAutoEnum):


 class CreditTransactionStatus(StrAutoEnum):


Q: this is always in sync with CreditTransactionStatus in resource_tracker_credit_transactions.py? do you test this?
Q: same thing with CreditClassification

sanderegg · 2025-01-19T20:41:30Z

...ervice-library/src/servicelib/rabbitmq/rpc_interfaces/resource_usage_tracker/service_runs.py

+    # pagination
+    offset: int = 0,
+    limit: int = 20,
+    # ordering
+    order_by: OrderBy | None = None,
 ) -> ServiceRunPage:


are these comments really useful? pagination, well I'd say it is pretty clear already. and ordering for something called order_by. I'd'say that is not necessary.

sanderegg · 2025-01-19T20:43:05Z

services/efs-guardian/src/simcore_service_efs_guardian/services/background_tasks.py

            )
-        except DBProjectNotFoundError as exc:
-            _logger.warning(
-                "Project %s not found, this should not happen, please investigate (contact MD)",
-                exc.msg_template,
+        except DBProjectNotFoundError:
+            _logger.info(
+                "Project %s not found. Removing EFS data for project {project_id} started",
+                project_id,
            )
+            await efs_manager.remove_project_efs_data(project_id)
        if (


this looks like a weird merge and looks wrong. can we discuss that one?

sanderegg · 2025-01-19T20:46:29Z

...ge-tracker/src/simcore_service_resource_usage_tracker/services/modules/db/service_runs_db.py

+    # attribute filtering
    service_run_status: ServiceRunStatus | None = None,
    started_from: datetime | None = None,
    started_until: datetime | None = None,
+    transaction_status: CreditTransactionStatus | None = None,
+    project_id: ProjectID | None = None,
+    # pagination
+    offset: int,
+    limit: int,
+    # ordering
    order_by: OrderBy | None = None,
-) -> list[ServiceRunWithCreditsDB]:
-    async with transaction_context(engine, connection) as conn:
-        query = (
+) -> tuple[int, list[ServiceRunWithCreditsDB]]:


so I see this is a new pattern.
I would propose the following instead of adding comments like these everywhere:

filter_by_service_run_status filter_by_started_from filter_by_started_until page_offset page_limit order_by

sanderegg · 2025-01-19T20:47:39Z

...ge-tracker/src/simcore_service_resource_usage_tracker/services/modules/db/service_runs_db.py

+    product_name: ProductName,
+    wallet_id: WalletID,
+    project_id: ProjectID,
+    transaction_status: CreditTransactionStatus | None = None,


and here you do not put #attribute filtering comment?
As I said I prefer not to have these kind of comments all over the place. usually that means that something is unclear.

sanderegg · 2025-01-19T20:54:50Z

services/resource-usage-tracker/tests/unit/with_dbs/test_api_credit_transactions.py

+    new_wallet_transaction = CreditTransactionCreateBody(
+        product_name=product_name,
+        wallet_id=_WALLET_ID_FOR_PAYING_DEBT__NOT_ENOUGH_CREDITS,
+        wallet_name="new wallet",


faker.name()

sanderegg · 2025-01-19T20:54:56Z

services/resource-usage-tracker/tests/unit/with_dbs/test_api_credit_transactions.py

+        wallet_id=_WALLET_ID_FOR_PAYING_DEBT__NOT_ENOUGH_CREDITS,
+        wallet_name="new wallet",
+        user_id=_USER_ID_1,
+        user_email="[email protected]",


faker.email()

sanderegg · 2025-01-19T20:55:47Z

...ces/resource-usage-tracker/tests/unit/with_dbs/test_process_rabbitmq_message_with_billing.py

@@ -240,7 +240,7 @@ async def test_process_event_functions(
        postgres_db, msg.service_run_id, modified_at
    )
    assert output.osparc_credits < first_credits_used
-    assert output.transaction_status == "BILLED"
+    assert output.transaction_status == "IN_DEBT"


why not using CreditTransactionStatus.IN_DEBT.value ?

sanderegg · 2025-01-19T20:57:23Z

services/web/server/src/simcore_service_webserver/projects/_states_handlers.py

@@ -127,6 +134,12 @@ async def open_project(request: web.Request) -> web.Response:
            ),
        )

+        await projects_service.raise_if_project_is_in_debt(


check_project_financial_status ? ;)

sanderegg · 2025-01-19T20:59:56Z

services/web/server/src/simcore_service_webserver/projects/_wallets_handlers.py

+
+class _PayProjectDebtBody(BaseModel):
+    amount: Annotated[Decimal, Field(lt=0)]
+    model_config = ConfigDict(extra="forbid")


Q: here I was under the assumption that we "accept clients that add stuff since we do not care and ignore whatever fields a client might add" and that we forbid for stuff we return. Or is there a reason to forbid any extra field?

daily work

8c81f52

Merge branch 'master' into changing-wallet-after-running-out-of-credits

d3d28fe

matusdrobuliak66 self-assigned this Jan 13, 2025

matusdrobuliak66 added this to the Singularity milestone Jan 13, 2025

matusdrobuliak66 added 3 commits January 14, 2025 14:01

openapi specs

e18149e

improbemtns

53a31cb

DB migration

bec976b

matusdrobuliak66 changed the title ~~✨ Introduce wallet exchange~~ ✨ Introduce wallet exchange 🗃️ Jan 14, 2025

matusdrobuliak66 and others added 3 commits January 14, 2025 15:51

fix test

9487856

Merge branch 'master' into changing-wallet-after-running-out-of-credits

634c8b9

Merge branch 'changing-wallet-after-running-out-of-credits' of github…

ed94f26

….com:matusdrobuliak66/osparc-simcore into changing-wallet-after-running-out-of-credits

matusdrobuliak66 added a:webserver issue related to the webserver service a:resource-usage-tracker resource usage tracker service labels Jan 16, 2025

matusdrobuliak66 and others added 9 commits January 16, 2025 13:24

Merge branch 'master' into changing-wallet-after-running-out-of-credits

fe27eed

Merge branch 'master' into changing-wallet-after-running-out-of-credits

d7c6e36

improvements and tests

32a63e9

improvements and tests

ec58d66

Merge branch 'master' into changing-wallet-after-running-out-of-credits

00e6d21

fix tests

c7f6bdb

adding unit tests

89bda75

adding test for open project

de3528a

openapi specs

80ccea2

matusdrobuliak66 marked this pull request as ready for review January 18, 2025 13:45

matusdrobuliak66 requested review from pcrespov, sanderegg, GitHK and giancarloromeo as code owners January 18, 2025 13:45

matusdrobuliak66 requested review from odeimaiz and bisgaard-itis January 18, 2025 13:46

matusdrobuliak66 added 2 commits January 18, 2025 15:00

fix

9b8685f

fix

61bbbf9

odeimaiz reviewed Jan 18, 2025

View reviewed changes

sanderegg reviewed Jan 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Introduce wallet exchange 🗃️ #7033

✨ Introduce wallet exchange 🗃️ #7033

matusdrobuliak66 commented Jan 13, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading

sonarqubecloud bot commented Jan 18, 2025

odeimaiz left a comment

sanderegg left a comment

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

sanderegg Jan 19, 2025

		@@ -38,16 +38,50 @@ class ServiceRunStatus(StrAutoEnum):


		class CreditTransactionStatus(StrAutoEnum):

✨ Introduce wallet exchange 🗃️ #7033

Are you sure you want to change the base?

✨ Introduce wallet exchange 🗃️ #7033

Conversation

matusdrobuliak66 commented Jan 13, 2025 • edited Loading

What do these changes do?

Related issue/s

How to test

Dev-ops checklist

codecov bot commented Jan 13, 2025 • edited Loading

Codecov Report

sonarqubecloud bot commented Jan 18, 2025

Quality Gate passed

odeimaiz left a comment

Choose a reason for hiding this comment

sanderegg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matusdrobuliak66 commented Jan 13, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading