Add AllocationDailyBillableUsage model and improve tests by jimmysway · Pull Request #317 · nerc-project/coldfront-plugin-cloud

jimmysway · 2026-05-26T21:18:05Z

Closes #292

…mmand Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

…mpty usage, remove duplicate print Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

…odel Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

…Model from the start Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Added tests to validate ORM layer and fetching layer that interacts with ORM

…ueries and standardizing string formatting in models and tests.

…nfo-database-table

reconfigured imports

…nfo-database-table

knikolla · 2026-06-01T15:12:08Z


    def ready(self):
        import coldfront_plugin_cloud.signals  # noqa: F401
+        import coldfront_plugin_cloud.models.daily_billable_usage  # noqa: F401


why is this being imported here?

I don't really understand everything here but I stumbled across this article when trying to figure out why .signals was being imported which led me to this article in the django documentation

https://docs.djangoproject.com/en/6.0/ref/applications/#how-applications-are-loaded

Where it says

Then Django attempts to import the models submodule of each application, if there is one.

You must define or import all models in your application’s models.py or models/init.py. Otherwise, the application registry may not be fully populated at this point, which could cause the ORM to malfunction.

Once this stage completes, APIs that operate on models such as get_model() become usable.

We have historically left the __init__.py blank and I don't know if this daily_billable_usage model is fundamentally different than the other ones because it is ORM related? I might need some help understanding this

Would adding the import in models/__init__.py instead of here work?

knikolla

Did a first pass at reviewing timeboxed to 25mins. Overall looks in the right direction. Please instead of creating a new command enhance the testing code to create the testing setup you need.

knikolla · 2026-06-04T20:21:46Z

+    return f"{today.year}-{today.month:02d}"
+
+
+class Command(BaseCommand):


Please don't create a new command for this. Instead look into either mocking the tests so that the values that you need are returned from the fictional CSV or sample values inserted into the DB.

Just to clarify, this seeding logic is primarily for testing of the downstream graphical UI elements in coldfront-nerc. I will find another way to insert sample values directly into the DB

knikolla · 2026-06-04T20:26:41Z


    def ready(self):
        import coldfront_plugin_cloud.signals  # noqa: F401
+        import coldfront_plugin_cloud.models.daily_billable_usage  # noqa: F401


Would adding the import in models/__init__.py instead of here work?

Copilot

Pull request overview

Adds persistent storage and retrieval for per-allocation daily billable usage by introducing an AllocationDailyBillableUsage model/table, wiring the existing fetch_daily_billable_usage command to upsert rows (and support deleting a day’s rows via --remove), and expanding test coverage plus a local-dev seeding command.

Changes:

Introduce AllocationDailyBillableUsage model + initial migration to store daily usage per (allocation, date, SU type).
Extend fetch_daily_billable_usage to write daily usage rows to DB and add --remove to delete rows for a given date.
Add DB-backed read helpers (billable_usage.py), a seed_daily_billable_usage command, and new/expanded unit tests.

Reviewed changes

Copilot reviewed 9 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/coldfront_plugin_cloud/tests/unit/test_fetch_daily_billable_usage.py	Adds tests covering DB insert/update behavior and the new `--remove` path.
src/coldfront_plugin_cloud/tests/unit/test_daily_billable_usage.py	New unit tests for DB read helpers and the new model’s constraints/behavior.
src/coldfront_plugin_cloud/models/daily_billable_usage.py	Adds the `AllocationDailyBillableUsage` Django model (FK to Allocation, date, su_type, value).
src/coldfront_plugin_cloud/migrations/0001_initial.py	Creates the new table, indexes, and unique constraint.
src/coldfront_plugin_cloud/migrations/init.py	Migration package marker.
src/coldfront_plugin_cloud/management/commands/seed_daily_billable_usage.py	New command to seed daily usage rows for local development/testing.
src/coldfront_plugin_cloud/management/commands/fetch_daily_billable_usage.py	Persists fetched usage to DB and adds `--remove` option to delete a date’s rows.
src/coldfront_plugin_cloud/billable_usage.py	New helper APIs to read daily usage (single day and date range) from the DB.
src/coldfront_plugin_cloud/apps.py	Imports the new model module during app initialization.
setup.cfg	Adds `django-model-utils` dependency for `TimeStampedModel`.
.gitignore	Ignores `*.db` files (likely local sqlite DB artifacts).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        time.sleep(0.02)
+        row.value = Decimal("150.00")
+        row.save()
+        row.refresh_from_db()


+    for su_type, amount in base.items():
+        step = _RAMP_STEP.get(su_type, 0)
+        ramped[su_type] = str(float(amount) + day_index * step)
+    return usage_models.UsageInfo(ramped)


+    Raises:
+        TypeError: If rows is None.
+        ValueError: If a row has an empty su_type.


jimmysway · 2026-06-05T14:01:21Z

I will go over all the comments but I also have one question right now there is a management/commands/fetch_daily_billable_usage.py which is the fetching code for S3 and also my newly written models/daily_billable_usage.py which is fetching from the database.

I find it confusing sometimes for which is which do you think I should rename these to distinguish between them better or is it fine.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI and others added 21 commits February 10, 2026 20:05

Initial plan

186c879

Add UsageInfo database model and update fetch_daily_billable_usage co…

96285b2

…mmand Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Add unit tests for database insertion and removal functionality

1815ed8

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Fix test assertions to use Decimal for database value comparisons

5e2c5d0

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Remove committed database file and update gitignore

960f7b6

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Fix trailing whitespace in code files

7b070eb

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Address code review feedback: improve related_name, add logging for e…

f1ac8f7

…mpty usage, remove duplicate print Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Rename UsageInfo to AllocationDailyBillableUsage and use TimeStampedM…

6762430

…odel Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Update migration to use AllocationDailyBillableUsage with TimeStamped…

92b4898

…Model from the start Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Remove confusing UsageInfoModel alias in tests

b162bb1

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Clean up redundant import alias in tests

c282a7b

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Address PR feedback: simplify code and use UsageInfo alias

befbe1c

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Keep both UsageInfo classes explicit to avoid confusion

75a1203

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Add explicit UsageInfo import to avoid unnecessary changes

6e68b5d

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Refactor tests: combine insertion/removal and add update test

b88e458

Co-authored-by: knikolla <4123322+knikolla@users.noreply.github.com>

Added methods to retrieve daily billable usage information.

06449fb

Added tests to validate ORM layer and fetching layer that interacts with ORM

Set PYTHONPATH to 'src' in CI scripts for functional and unit tests

01edadc

Refactor code for consistency and readability by simplifying filter q…

3a6eee1

…ueries and standardizing string formatting in models and tests.

Merge remote-tracking branch 'upstream/main' into copilot/add-usage-i…

f4e950e

…nfo-database-table

merged into upstream/main

6bb794e

reconfigured imports

fixed imports

0553fa8

QuanMPhm mentioned this pull request May 28, 2026

Resolve breaking changes by kubernetes package #318

Merged

fixed ruff

8781083

jimmysway force-pushed the copilot/add-usage-info-database-table branch from 01b3089 to 8781083 Compare May 28, 2026 20:07

jimmysway added 2 commits May 28, 2026 16:22

Merge remote-tracking branch 'upstream/main' into copilot/add-usage-i…

7aaa12e

…nfo-database-table

fix linter errors

392a176

jimmysway mentioned this pull request May 29, 2026

feat(#178): Responsiveness & interactivity for PI Dashboard nerc-project/coldfront-nerc#180

Open

knikolla reviewed Jun 1, 2026

View reviewed changes

knikolla requested changes Jun 4, 2026

View reviewed changes

knikolla requested a review from Copilot June 4, 2026 20:29

Copilot started reviewing on behalf of knikolla June 4, 2026 20:29 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

Potential fix for pull request finding

805e059

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add AllocationDailyBillableUsage model and improve tests#317

Add AllocationDailyBillableUsage model and improve tests#317
jimmysway wants to merge 25 commits into
nerc-project:mainfrom
jimmysway:copilot/add-usage-info-database-table

jimmysway commented May 26, 2026

Uh oh!

knikolla Jun 1, 2026

Uh oh!

jimmysway Jun 1, 2026 •

edited

Loading

Uh oh!

knikolla Jun 4, 2026

Uh oh!

knikolla left a comment

Uh oh!

knikolla Jun 4, 2026

Uh oh!

jimmysway Jun 5, 2026 •

edited

Loading

Uh oh!

knikolla Jun 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

jimmysway commented Jun 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		return f"{today.year}-{today.month:02d}"


		class Command(BaseCommand):

Conversation

jimmysway commented May 26, 2026

Uh oh!

knikolla Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

jimmysway Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knikolla Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

knikolla left a comment

Choose a reason for hiding this comment

Uh oh!

knikolla Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

jimmysway Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knikolla Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

jimmysway commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jimmysway Jun 1, 2026 •

edited

Loading

jimmysway Jun 5, 2026 •

edited

Loading

jimmysway commented Jun 5, 2026 •

edited

Loading