-
Notifications
You must be signed in to change notification settings - Fork 106
Adding the inference grant to the Claim #1139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for adding missing bits.
… db with metrics and app name
I added Gateway specific guid IDs. These are used as options when connecting to the psql db. A psql connection is required for when calling |
auth/grants.go
Outdated
|
||
type InferenceGrant struct { | ||
// Admin grants to all inference features (LLM, STT, TTS) | ||
Admin bool `json:"admin,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we make this more nuanced vs having to rely on the "admin" permission?
i.e. does it make sense to have separate permission tokens for diff services? i.e. either:
llm: bool
tts: bool
stt: bool
or maybe a single "perform" permission to allow performing an inference? (as opposed to admin could mean update settings or other features)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to add the finer grain permissions but decided to go with the simpler approach since we can add those permissions later. Happy to add them now though. Also, I like the name perform
better, will change it.
utils/guid/id.go
Outdated
CloudAgentVersionPrefix = "CAV_" | ||
CloudAgentSecretPrefix = "CAS_" | ||
CloudAgentWorkerPrefix = "CAW_" | ||
CloudAgentGatewayPrefix = "CAG_" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the idea that for each inference request we'd have an ID?
if so could we just call it CloudInference
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These values are prefixes for node ids. The node ids are used as labels for the DB prometheus metrics. Happy to rename it but this seems to be the convention I noticed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed these values since the gateway no longer needs a DB connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This grant will be used by the inference gateway for authentication.