search

appuio / appuio-cloud-reporting

Reporting for APPUiO Cloud
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Introduce Display Names for queries #65

Closed glrf closed 2 years ago

glrf commented 2 years ago

Display names are short, human readable names that can be use by the ERP adapter to generate readable names without having to hardcode display names for every product.

This grew out of https://github.com/vshn/appuio-odoo-adapter/pull/41 where I'm struggling to generate usable invoices with the data I have. I'm not 100% sure this is the right approach, so I'm open for other input.

Checklist

bastjan commented 2 years ago

I'm not sure if we should start with non-translatable names here. Other adapters might use other names for things (or need them shorter, longer, ...) and the plan is to read the templates from a config map, so they can be quickly updated.

I'd prefer a lookup on the https://github.com/vshn/appuio-odoo-adapter side until we have something better.

$queryShorts := dict "appuio_cloud_memory_subquery_cpu_request" "CPU Requests"

{{ . | get $queryShorts }}: {{.QuantityAvg | perMinute | printf "%.2f"}} {{.Unit}}
glrf commented 2 years ago

Maybe you're right. I also thought about solving it that way (that's why I opened https://github.com/appuio/appuio-cloud-reporting/pull/64).

On the other hand a display_name is something that nearly every ERP adapter will need. But you're right they might need names with different lengths.. I'm really not sure which solution is better

corvus-ch commented 2 years ago

I prefer to keep the representation side in the ERP adapter. Please handle those labels in the template.

Do the individual sub results have a deterministic key? If not, I would change it that way. The template, instead of iterating over the list, then explicitly pulls in the values.

CPU requests: {{ .SubItems.appuio_cloud_cpu_requests.QuantiyAvg | perMinute | printf "%.2f" }} {{ .Unit }}

Or something along that line.

glrf commented 2 years ago

Alright we'll handle this in the ERP adapter. And if you prefer that we can change the SubItems slice to a map. No strong feelings there.