chore: add API key ID to interceptions #20513
Conversation
|
|
Thank you @dannykopping for pointing out that changes in aibridge repo are not necessary. |
coderd/database/migrations/000396_add_aibridge_interceptions_api_key_id.up.sql
Show resolved
Hide resolved
| require.Equal(t, "openai", intc0.Provider) | ||
| require.Equal(t, "gpt-4.1", intc0.Model) | ||
| require.True(t, intc0.EndedAt.Valid) | ||
| require.WithinDuration(t, dbtime.Now(), intc0.EndedAt.Time, 10*time.Second) |
There was a problem hiding this comment.
Why dbtime.Now() vs intc0.StartedAt?
Why 10s? Can we not just validated that ended at > started at?
There was a problem hiding this comment.
I'm not sure if there is a good way to verify time here. 10s is just a arbitrary number so it is close enough to believe that time is set correctly to Now() but also flake safe.
If we want to check time here I probably should check StartedAt using the same condition.
There was a problem hiding this comment.
What are you actually testing here, though? Just that the values were stored correctly?
There was a problem hiding this comment.
Yeah, testing if values are correctly stored / passed between all http/rpc/db layers.
Also I think this is the only place that tests translator (enterprise/x/aibridged/translator.go). All tests passed when I forgot to update translator until I've added check here which scared me a bit.
There was a problem hiding this comment.
OK, in that case I don't see why you need to test the time in this way; all you need to validate is that start/end time are set and that end > start, no?
There was a problem hiding this comment.
Changed to StartedAt < EndedAt && EndedAt - StartedAt < 5s
Just checking Started < Ended would not catch edge case where StartedAt would be set to zero value.
badad42 to
cb7afb0
Compare
| require.Equal(t, "openai", intc0.Provider) | ||
| require.Equal(t, "gpt-4.1", intc0.Model) | ||
| require.True(t, intc0.EndedAt.Valid) | ||
| require.WithinDuration(t, dbtime.Now(), intc0.EndedAt.Time, 10*time.Second) |
There was a problem hiding this comment.
OK, in that case I don't see why you need to test the time in this way; all you need to validate is that start/end time are set and that end > start, no?
5f1b9c4 to
78210b9
Compare
3e33a1c to
0d59950
Compare
0d59950 to
d6e752e
Compare
Adds APIKeyID to interceptions.
Needed for tracking API key usage with bridge.
fixes #20001