Conversation
tmaptible
force-pushed
the
sc-35229
branch
2 times, most recently
from
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
eabruzzese
left a comment
There was a problem hiding this comment.
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.
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.
(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.
Do we know why this is the case? Just curious.
tmaptible
changed the title
3 participants
No description provided.