search

neondatabase / autoscaling

Postgres vertical autoscaling in k8s
Apache License 2.0
142 stars 16 forks source link

agent/billing: support Azure Blob Storage to store consumption events #994

Closed antonyc closed 4 days ago

antonyc commented 1 week ago

This enables billing package to persist consumption events in Azure Blob storage. Tests are done with Azurite ^1 docker container through gnomock library.

https://github.com/neondatabase/cloud/issues/14786

sharnoff commented 1 week ago

Thanks for tackling this! Haven't looked at the code yet, but a request about the PR title:

Could you rename the PR to the format component/subsystem: title (e.g. here, "agent/billing: use Azure ...") (Here though, I'd also note that the PR doesn't switch the autoscaler-agent to using Azure, it just adds support - IIUC?)

Probably we should have a workflow to enforce this -- I just haven't gotten around to opening an issue for it

antonyc commented 1 week ago

Could you rename the PR to the format component/subsystem: title (e.g. here, "agent/billing: use Azure ...")

I like https://www.conventionalcommits.org/en/v1.0.0/#summary, have you looked at it? There is a lot of tooling around it.

antonyc commented 1 week ago

(Here though, I'd also note that the PR doesn't switch the autoscaler-agent to using Azure, it just adds support - IIUC?)

It does, did I miss something?

sharnoff commented 1 week ago

It does, did I miss something?

Yeah, we'll need to enable it from the deploy side -- see e.g. here. (in short: the YAMLs produced by the release don't have billing enabled, because there's nowhere to configure them; so enabling is done further downstream)

antonyc commented 1 week ago

Yeah, we'll need to enable it from the deploy side -- see e.g. here.

There is a PR in review (it's on you), and then the next followup PR that has not been published yet enables it for Azure.

Omrigan commented 1 week ago

I'd also note that the PR doesn't switch the autoscaler-agent to using Azure, it just adds support - IIUC?

@antonyc +1 for s/use/support/ in PR subj.

antonyc commented 5 days ago

@Omrigan please have a look.