Specialised handling for griogrits, credit code padding and emre_keskin credit application by jimmysway · Pull Request #270 · CCI-MOC/invoicing

jimmysway · 2026-02-24T21:03:11Z

Closes #268.

Added logic for emre harvard Openstack Storage free storage Added test case for emre Harvard Openstack Storage handling Closes CCI-MOC#268

… to Billable Added test case for grio-grits move from Non-Billable to Billable

knikolla

I don't see it as being added to the list of Processors in process_report.py

knikolla · 2026-03-04T19:48:52Z

+        self.data.loc[rule_mask, invoice.PI_BALANCE_FIELD] = 0
+        self.data.loc[rule_mask, invoice.BALANCE_FIELD] = 0
+
+    def _apply_griot_grits_billable(self):


@joachimweyl why is this project not automatically classified as billable and why does it need a special rule?

The whole test cluster and barcelona cluster are non-billable by defult in the hopes that eventually we will not need to use these as "produciton". therefore to make anything from these 2 clusters they need to be made billable manually.

So this is a billable project in a non billable cluster? Sounds like a better place to handle this would be introducing a new field in the projects in the projects.yaml file.

@QuanMPhm cc.

I believe a new override field, is_billable, that indicates if the project is billable for all, or specific clusters, would suffice?

I believe a new override field, is_billable, that indicates if the project is billable for all, or specific clusters, would suffice?

Yes. If the override field exists the billing status of the project will be the value of the field.

If the field is missing from the project entry, and there is a project entry in projects.yaml, it will be assumed as False.

Otherwise, if there is no entry in projects.yaml, it will take the value of the PI or cluster.

The whole non-billable-projects repo historically has been made up of non-billable data that is how it is understood.

Hence why I suggested at some point in the near-future renaming that repository into invoicing-private-data, as that is what it has become.

If we add this billable project into the projects.yaml it won't necessarily be easy to track down what has happened to this project specifically.

I strongly disagree. It is much more harder to track down what is happening when you need to track down and read the code to see that you are setting the specific flag in a specific column, rather than having a readable and plain English is_billable: True in a YAML file.

Special cases imho are special because they need to be traced, we need to know what is going on.

This is not the first, nor the last special case that we have handled.

If we allow every special cases that we encounter to be one-offs, without ever generalizing, we're just going to be having a long list of one-off processors and then it will be truly lost and disappear in the list of processors.

What I am proposing here is a solution that allows this to not be a one-off anymore, since the difficulty of generalizing both cases in this PR is low and these cases become supported by the schema of the YAML files here.

I’m not opposed to encoding this in YAML instead of code. My hesitation is specifically that projects.yaml and the surrounding repo have historically meant “nonbillable data,” so adding is_billable: true changes the meaning of that data source in a way that may confuse future operators. I understand that we will be changing the name of the repo in the future but those who have come across the projects.yaml have always understood it as explicity non-billable data.

I don't mean having projects ids randomly hardcoded in is good and I completely agree with the YAML plain english way of organising this information.

I propose that we introduce maybe a separate file perhaps called billing_overrides.yaml, or something of that nature so that projects.yaml can remain explicitly non-billable information as is familiar with everyone.

I propose that we introduce maybe a separate file perhaps called billing_overrides.yaml, or something of that nature so that projects.yaml can remain explicitly non-billable information as is familiar with everyone.

No.

projects.yaml is already a billing override, just with an implicit is_billable: False to all the projects listed there. Schema changes happen naturally, as is happening here, and this schema is the best place to accommodate this new feature. The default behavior will be preserved and an explicit new behavior will be introduced, so there are no concerns for confusion or breaking changes. Plus, we are the operators, so don't worry about future ones.

A new billing_overrides.yaml file that is separate from an already existing billing override file will only cause more confusion and split the same kind of schema into two separate places.

I have renamed cci-moc/non-billable-projects to cci-moc/invoicing-private-data

I would please like to no longer debate this further.

https://github.com/CCI-MOC/invoicing-private-data/issues/89

This is a link to the issue

#274 This is a link to the updated handling

knikolla · 2026-03-04T21:21:50Z

+
+@dataclass
+class SpecialBillingRulesProcessor(processor.Processor):
+    _EMRE_EMAIL = "emre_keskin@harvard.edu"


Instead of hardcoding the email that is not being billed for a specific resource, perhaps we can turn pi.txt in the non-billable-projects repo into a YAML file (as we did with projects.yaml) that contains for each PI in the list a field named specific-non-billed-su-types (or something similar) and takes a list of SU types to not bill the PI for.

I strongly dislike this kind of hardcoding of user data, be it emails, su types or specific projects into the code.

@QuanMPhm @jimmysway

I'm fine with this suggestion. @jimmysway Do you have other suggestions, and do you want to implement this change in nerc-rates?

I don't think nerc-rates is the right place to put user information. That's why I mentioned non-billable-projects, which perhaps is about time we rename it to invoicing-private-data.

I'm confused to where I am supposed to include the email information.

I don't think nerc-rates is the right place to put user information. That's why I mentioned non-billable-projects, which perhaps is about time we rename it to invoicing-private-data.

Ah sorry, I meant non-billable-projects. First time I got them mixed up :P

So do you want to move this logic to the ValidateBillablePIsProcessor? @knikolla Or just load the PI.yaml into the special processor. I still maintain that special cases need to be handled in one spot

So do you want to move this logic to the ValidateBillablePIsProcessor?

Yes

I still maintain that special cases need to be handled in one spot

They wouldn't be special cases anymore, but supported by the schema of the respective YAML files. @jimmysway please trust my judgment.

I will make that PR now to modify pi.txt into PR.yaml

https://github.com/CCI-MOC/non-billable-projects/pull/88

here is the PR

jimmysway · 2026-03-05T18:45:13Z

I don't see it as being added to the list of Processors in process_report.py

There seems to be a lot of special one off cases that may grow in the future. I thought implementing a new processor would firstly, allow future one off cases to be implemented and secondly it one off cases would be centralized inside one distinct processor so you don't have to hunt for those cases in the future, it is clear what the processor is doing so the logic is very auditable.

@knikolla @QuanMPhm

In any case, this will create a sort of framework or guide for future implementation of one of cases.

knikolla · 2026-03-05T18:48:12Z

I don't see it as being added to the list of Processors in process_report.py

There seems to be a lot of special one off cases that may grow in the future. I thought implementing a new processor would firstly, allow future one off cases to be implemented and secondly it one off cases would be centralized inside one distinct processor so you don't have to hunt for those cases in the future, it is clear what the processor is doing so the logic is very auditable.

@knikolla @QuanMPhm

@jimmysway

I'm commenting on the fact that since this isn't being added to the list of processors in process_report.py, it won't actually execute as part of the pipeline. My comment doesn't touch about any other aspect of this PR.

EDIT: To further edit and clarify, this must be added to that list for it to execute.

jimmysway · 2026-03-05T18:48:57Z

I don't see it as being added to the list of Processors in process_report.py

There seems to be a lot of special one off cases that may grow in the future. I thought implementing a new processor would firstly, allow future one off cases to be implemented and secondly it one off cases would be centralized inside one distinct processor so you don't have to hunt for those cases in the future, it is clear what the processor is doing so the logic is very auditable.
@knikolla @QuanMPhm

@jimmysway

I'm commenting on the fact that since this isn't being added to the list of processors in process_report.py, it won't actually execute as part of the pipeline. My comment doesn't touch about any other aspect of this PR.

I misunderstood my apologies

QuanMPhm · 2026-03-05T22:53:52Z

@jimmysway Sorry for being late to respond. To clarify your confusion here and here, @knikolla is suggesting that we introduce new fields in the projects.yaml file to remove the need of hardcoding information as was the case in this PR. If there are new billing behaviors (i.e a project must always be billable regardless of clusters or other criterias), it should be implemented in a way that is more systematic (the code should be applicable regardless of PI/project) and centralized (info on the "billability" of projects should be stored in one place, preferably one friendly to non-developers like @joachimweyl. The projects.yaml meets these criterias).

My suggestion is that you update the model for project.yaml to include the new fields @knikolla suggested, then update the Billable Processor to handle the fields accordingly (along with potentially other files).

Make a PR to non-billable-projects first to propose your new fields and their documentation.

Does this clarify your confusion?

QuanMPhm · 2026-03-19T13:19:06Z

If you believe you will not continue development on this PR, can you close it?

QuanMPhm · 2026-03-31T14:28:46Z

Closed as it is superseded by #279 and #275

jimmysway added 2 commits February 24, 2026 12:25

Added a special processing for one off or specialised handling

495ec3c

Added logic for emre harvard Openstack Storage free storage Added test case for emre Harvard Openstack Storage handling Closes CCI-MOC#268

Added functionality to move griot-grits-aa488b rows from Non-Billable…

6563d06

… to Billable Added test case for grio-grits move from Non-Billable to Billable

jimmysway requested review from QuanMPhm and knikolla February 24, 2026 21:03

jimmysway changed the title ~~Feature/268 special handling~~ Specialised handling for griogrits, credit code padding and emre_keskin storage Mar 3, 2026

jimmysway changed the title ~~Specialised handling for griogrits, credit code padding and emre_keskin storage~~ Specialised handling for griogrits, credit code padding and emre_keskin credit application Mar 3, 2026

knikolla requested changes Mar 4, 2026

View reviewed changes

knikolla reviewed Mar 4, 2026

View reviewed changes

QuanMPhm closed this Mar 31, 2026

Uh oh!

Conversation

jimmysway commented Feb 24, 2026 • edited by QuanMPhm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

knikolla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jimmysway Mar 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!

Choose a reason for hiding this comment

Uh oh!

jimmysway Mar 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

QuanMPhm commented Mar 5, 2026

Uh oh!

QuanMPhm commented Mar 19, 2026

Uh oh!

QuanMPhm commented Mar 31, 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 commented Feb 24, 2026 •

edited by QuanMPhm

Loading

jimmysway Mar 5, 2026 •

edited

Loading

jimmysway Mar 5, 2026 •

edited

Loading

jimmysway commented Mar 5, 2026 •

edited

Loading

knikolla commented Mar 5, 2026 •

edited

Loading

jimmysway commented Mar 5, 2026 •

edited

Loading

QuanMPhm commented Mar 31, 2026 •

edited

Loading