- Notifications
You must be signed in to change notification settings - Fork1k
feat: add agent metadata#6614
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
thanks kyle
What do you mean by dynamic metadata? |
@matifali — see screenshot I added to original post. |
| PostLifecycle(ctx context.Context,state agentsdk.PostLifecycleRequest)error | ||
| PostAppHealth(ctx context.Context,req agentsdk.PostAppHealthsRequest)error | ||
| PostStartup(ctx context.Context,req agentsdk.PostStartupRequest)error | ||
| PostMetadata(ctx context.Context,keystring,req agentsdk.PostMetadataRequest)error |
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.
If someone has a lot of metadata, I could see us wanting to debounce some of the requests... e.g. 5 things polling each second * 1000 workspaces = alot of requests
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 considered this but didn't want to pursue optimizing it immediately because the feature is very optional and there's a clear path to adding a WebSocket endpoint to improve performance when the time comes.
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.
A WebSocket endpoint would add substantial complexity (making this harder to review...) and bug risk.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| ifmd.Interval==0 { | ||
| continue | ||
| } | ||
| ifcollectedAt.Add( |
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.
How is this possible?
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.
See new comment
Uh oh!
There was an error while loading.Please reload this page.
@kylecarbs — made some minor changes, and fixed one concurrency bug... you should take another look at the short diff. |
| // We send the result to the channel in the goroutine to avoid | ||
| // sending the same result multiple times. So, we don't care about | ||
| // the return values. | ||
| flight.DoChan(md.Key,func() (interface{},error) { |
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.
Why useDoChan instead ofDo?
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.
It's to not block the loop, and a bit cleaner than wrapping the whole thing in a goroutine.
Uh oh!
There was an error while loading.Please reload this page.
Resolves#3480
Now
Later