Don't deploy billing prometheus rule for tests #272
Conversation
Kidswiss
requested review from
a team,
TheBigLee,
wejdross and
zugao
and removed request for
a team
| @@ -22,6 +23,11 @@ func CreateBillingRecord(ctx context.Context, svc *runtime.ServiceRuntime, comp | |||
| log := controllerruntime.LoggerFrom(ctx) | |||
| log.Info("Enabling billing for service", "service", comp.GetName()) | |||
|
|
|||
| if comp.GetClaimNamespace() == "vshn-test" { | |||
There was a problem hiding this comment.
do we want to parametrize it via component, so it's not that visible to the whole world?
There was a problem hiding this comment.
If we parameterize it in the component and then set the default to vshn-test then the whole world will see it all the same 🤷
There was a problem hiding this comment.
Let me change a bit the argument. Instead of burring this string inside the code here. Wouldn't it be better to set it into component, under billing? I mean in 6 months we will forget about this.
There was a problem hiding this comment.
That's a better argument :D
However then we have yet another parameter in the already very bloated component.
There was a problem hiding this comment.
I would still move it there, we will know that it exists instead of forgetting about it. There's no escape from bloating the component. And this is not the solution to un-bloat it, we will have to come up with smth else. I would really encourage to create a parameter there.
Update: I understand it's a pain in the ... but we need to better track these things otherwise everything is out of control
There was a problem hiding this comment.
I've made it configurable now.
But making a value that could be a constant value configurable just so we don't forget about it also seems wrong. It's not like we remember all the stuff that's in the component as well.
There was a problem hiding this comment.
I would go to the component to search for smth first instead of the go code.
There was a problem hiding this comment.
The right place for that would be the docs, but noone ever reads them, so I also stopped documenting...
There was a problem hiding this comment.
my idea was to set a global commodore parameter, this way we would keep namespace name for ourselfs
This will not deploy any billing data for instances where the claim resides in the `vshn-test` namespace.
Kidswiss
force-pushed
the
add/ignore-vshn-test-billing
branch
from
bd423f5 to
3908409
Compare
Summary
This will not deploy any billing data for instances where the claim resides in the
vshn-testnamespace.Checklist
bug,enhancement,documentation,change,breaking,dependencyas they show up in the changelog