-
Notifications
You must be signed in to change notification settings - Fork 37
sc-35229: Aptible Managed AI #392
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
base: master
Are you sure you want to change the base?
Conversation
8d7ed10 to
dcc5223
Compare
|
Removing myself for now, as it looks like this requires a merge conflict to be resolved, updated API gem to be released and updated here, etc. |
eabruzzese
left a comment
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.
Everything's working for me. Added a couple of notes/questions out of curiosity, but it does what it says on the tin, and I think that's good enough for the internal beta.
Might be worth having someone more knowledgeable about the intended patterns in the CLI take a look as well, though.
Commenting for now since the tests haven't passed, but tentatively approved.
| # URL-safe base64 encode the note for safe transport to deploy-api | ||
| # deploy-api will validate and encrypt it before storing in LLM Gateway | ||
| require 'base64' | ||
| opts[:note] = Base64.urlsafe_encode64(options[:note], padding: true) |
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.
Just curious: why do we base64-encode here? We say "safe transport", but I'm not sure I understand how it helps, since we're POSTing a JSON payload over HTTPS. It looks like the encryption strategy in deploy-api involves base64-encoding the payload, but it seems to me like it would be better to have deploy-api do that instead of the client?
| if ENV['APTIBLE_DEBUG'] == 'DEBUG' | ||
| begin | ||
| tokens_array = tokens.map(&:body) | ||
| CLI.logger.warn "GET /accounts/#{account.id}/ai_tokens response: #{JSON.pretty_generate(tokens_array)}" | ||
| rescue StandardError | ||
| CLI.logger.warn "GET /accounts/#{account.id}/ai_tokens response: #{tokens.map(&:body).inspect}" | ||
| end | ||
| end |
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.
(nit for later): there are a lot of these kinds of debug blocks, and I wonder if there's a more generic place to put debug logic related to API calls?
| telemetry(__method__, options.merge(id: id)) | ||
|
|
||
| # GET /ai_tokens/:id via HAL | ||
| # Must set root URL explicitly for the request to work |
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.
Do we know why this is the case? Just curious.
No description provided.