Conversation
|
⚪
🟢 |
|
⚪
🟢 |
There was a problem hiding this comment.
Pull request overview
This PR enhances the MVP “meta” service capabilities reporting by tracking registered HTTP handlers (with versions) and serving them via the /capabilities endpoint, while also refactoring handler registration to go through a shared helper.
Changes:
- Added
TMetaCapabilitiesto collect{path -> version}and serialize it to JSON. - Introduced
TMVP::RegisterMetaHandler()to both register HTTP handlers and record their capability/version. - Updated
/capabilitieshandler to return real computed capabilities instead of a stub response.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
ydb/mvp/meta/mvp.h |
Adds TMetaCapabilities storage to TMVP and declares RegisterMetaHandler() helper. |
ydb/mvp/meta/meta.cpp |
Refactors handler registrations through RegisterMetaHandler() and wires /capabilities to use the collected capabilities. |
ydb/mvp/meta/meta_capabilities.h |
Implements capabilities collection + JSON serialization and updates the /capabilities handler to return computed data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ae4921f to
9ae3678
Compare
|
⚪
🟢 |
|
⚪
🟢 |
9ae3678 to
8696b18
Compare
|
⚪
🟢 |
|
⚪
🟢 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8696b18 to
3d0a4a6
Compare
|
⚪
🟢 |
|
⚪
🟢 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| explicit THandlerActorMetaCapabilities(std::shared_ptr<const TMetaCapabilities> capabilities) | ||
| : TBase(&THandlerActorMetaCapabilities::StateWork) | ||
| , Capabilities(std::move(capabilities)) | ||
| {} | ||
|
|
||
| void Handle(const NHttp::TEvHttpProxy::TEvHttpIncomingRequest::TPtr& event, const NActors::TActorContext& ctx) { | ||
| // Stub endpoint for UI compatibility so clients do not receive 404 while capabilities are not implemented yet. | ||
| static constexpr TStringBuf ResponseBody = "{\n \"Capabilities\": {}\n}\n"; | ||
| auto response = event->Get()->Request->CreateResponseOK(ResponseBody, "application/json; charset=utf-8"); | ||
| NJson::TJsonValue responseBody(NJson::JSON_MAP); | ||
| responseBody["Capabilities"] = Capabilities->ToJson(); | ||
|
|
There was a problem hiding this comment.
THandlerActorMetaCapabilities dereferences Capabilities unconditionally. Since the ctor accepts a std::shared_ptr it’s possible to accidentally pass nullptr and get a crash on the first request; consider enforcing non-null in the constructor (e.g., assert/abort) or storing by value/snapshot to make the handler self-contained.
Changelog entry
This PR enhances the MVP “meta” service capabilities reporting by tracking registered HTTP handlers (with versions) and serving them via the /capabilities endpoint, while also refactoring handler registration to go through a shared helper.
Changelog category
Description for reviewers
...