Conversation
seanshi-scale
left a comment
There was a problem hiding this comment.
I'm not super confident we should be exposing notions e.g. user id + api key for the public repo, e.g. this functionality may belong better somewhere else (e.g. model-engine-internal)
Hmm I suppose that may be true. If we're hosting internally, we don't need to mention API keys here because the rest of our stack handles that; if we're deploying in customer VPCs, we rely on them to manage API keys and auth. The main difference is that the EGP APIs do need to do this, because they're more of a fully fledged platform product that needs to manage its own API keys. cc @eric-scale |
I guess that I'm just acknowledging the underlying |
yixu34
left a comment
There was a problem hiding this comment.
【we love dependency injection】
| from model_engine_server.domain.gateways.monitoring_metrics_gateway import MonitoringMetricsGateway | ||
|
|
||
|
|
||
| class DatadogMonitoringMetricsGateway(MonitoringMetricsGateway): |
There was a problem hiding this comment.
did this file get moved to internal?
There was a problem hiding this comment.
I suppose that dependencies on public vendors (e.g. Datadog) can be put in our public repo. It's the Scale-specific bits that need to be internal.
There was a problem hiding this comment.
yup, now a plugin
first step in API key migration - chain functions tried