6e7148a945
Merge pull request 'adr: Add ADR on multi tenancy using namespace based customer isolation' (#41) from adr/multi-tenancy into master
I like this solution for now, I think the best solution would be a dedicated cluster per tenant but it makes sense why we can't. I think the proposed solution works for our current scale.…
You missed the point of using a trait here. See my comment and refactor accordingly.
This defeats the purpose of using a trait. Also prevents extensibility by external users of Harmony.
I feel like this should not be public. Our public APIs should go through Scores, Topologies and Inventories. What's the reasonning here?
It's exposed through the score, and if a score…
I feel like this should not be public. Our public APIs should go through Scores, Topologies and Inventories. What's the reasonning here?
Yeah, these two functions here discord_alert_builder and slack_alert_builder here should be implementations of a trait.
Since we're supporting a list, shouldn't we use filter_map instead so we can handle all the instances at once?
The name MonitoringAlertingStackScore bothered me all along but I couldn't figure out why. Now I did :
No need for this monitoring_stack: Stack field for now. This is a case of YAGNI. We support only one type and I don't see in the very short term a use case that would force us.
What is service_name used for? As this is user facing, a bit of rust doc (with triple slashes ///) to describe how to use would be useful here.