1845 daps refactor repo server to use proto3#1846
Merged
JoshuaSBrown merged 10 commits into1843-DAPS-refactor-core-server-to-use-proto3from Feb 7, 2026
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @JoshuaSBrown, your pull request is larger than the review limit of 150000 diff characters
….com:ORNL/DataFed into 1845-DAPS-refactor-repo-server-to-use-proto3
JoshuaSBrown
changed the base branch from
1843-DAPS-refactor-core-server-to-use-proto3
to
devel
JoshuaSBrown
changed the base branch from
devel
to
1843-DAPS-refactor-core-server-to-use-proto3
JoshuaSBrown
added
Component: Core
Collaborator
Author
|
@sourcery-ai review |
….com:ORNL/DataFed into 1845-DAPS-refactor-repo-server-to-use-proto3
Contributor
Reviewer's GuideRefactors the repo server to use the unified proto3 envelope/message definitions instead of protocol-ID–based SDMS_Anon/Auth protos, and updates error codes and version reporting to match the new proto3 constants and layout. Sequence diagram for proto3 envelope-based request handling in RepoServersequenceDiagram
actor Client
participant RepoServer
participant RequestWorker
participant ProtoBufMap
participant MsgMapper
participant Handler as RequestHandler
Client->>RepoServer: Send Envelope(message_type, payload)
RepoServer->>RequestWorker: Deliver Envelope
activate RequestWorker
RequestWorker->>MsgMapper: getMessageType(envelope_field_name)
MsgMapper-->>RequestWorker: msg_type (uint16)
RequestWorker->>ProtoBufMap: toString(msg_type)
ProtoBufMap-->>RequestWorker: msg_name
Note right of RequestWorker: Log received message using msg_name
alt handler_registered
RequestWorker->>Handler: invoke handler(msg_request)
Handler-->>RequestWorker: response Envelope
else no_handler
RequestWorker-->>RequestWorker: create NackReply
Note right of RequestWorker: err_code BAD_REQUEST
end
RequestWorker-->>RepoServer: response Envelope
RepoServer-->>Client: Send response
deactivate RequestWorker
Updated class diagram for RequestWorker and proto3 message handlingclassDiagram
class RequestWorker {
- static map~uint16_t, msg_fun_t~ m_msg_handlers
- unique_ptr~MsgMapper~ m_msg_mapper
- MessageFactory m_msg_factory
- bool m_run
+ void wait()
+ void setupMsgHandlers()
+ void workerThread(LogContext log_context)
+ unique_ptr~IMessage~ procVersionRequest(unique_ptr~IMessage~ msg_request)
+ unique_ptr~IMessage~ procDataDeleteRequest(unique_ptr~IMessage~ msg_request)
+ unique_ptr~IMessage~ procDataGetSizeRequest(unique_ptr~IMessage~ msg_request)
+ unique_ptr~IMessage~ procPathCreateRequest(unique_ptr~IMessage~ msg_request)
+ unique_ptr~IMessage~ procPathDeleteRequest(unique_ptr~IMessage~ msg_request)
}
class MsgMapper {
+ uint16_t getMessageType(string msg_name)
}
class ProtoBufMap {
+ string toString(uint16_t msg_type)
}
class MessageFactory {
+ unique_ptr~IMessage~ createResponseEnvelope(IMessage request)
}
class IMessage {
<<interface>>
}
class AckReply {
+ void set_err_code(int code)
+ void set_err_msg(string msg)
}
class NackReply {
+ void set_err_code(int code)
+ void set_err_msg(string msg)
}
class VersionReply {
+ void set_release_year(int year)
+ void set_release_month(int month)
+ void set_release_day(int day)
+ void set_release_hour(int hour)
+ void set_release_minute(int minute)
+ void set_api_major(int major)
+ void set_api_minor(int minor)
+ void set_api_patch(int patch)
+ void set_component_major(int major)
+ void set_component_minor(int minor)
+ void set_component_patch(int patch)
}
namespace SDMS_release {
class release_constants {
<<static>>
+ int YEAR
+ int MONTH
+ int DAY
+ int HOUR
+ int MINUTE
}
}
namespace SDMS_protocol_version {
class protocol_version_constants {
<<static>>
+ int MAJOR
+ int MINOR
+ int PATCH
}
}
namespace SDMS_repository_version {
class repository_version_constants {
<<static>>
+ int MAJOR
+ int MINOR
+ int PATCH
}
}
RequestWorker --> MsgMapper : uses for msg_type lookup
RequestWorker --> ProtoBufMap : uses for msg_type logging
RequestWorker --> MessageFactory : creates response envelopes
RequestWorker ..> IMessage : processes
MessageFactory ..> IMessage
AckReply ..|> IMessage
NackReply ..|> IMessage
VersionReply ..|> IMessage
RequestWorker --> AckReply
RequestWorker --> NackReply
RequestWorker --> VersionReply
RequestWorker --> release_constants : uses
RequestWorker --> protocol_version_constants : uses
RequestWorker --> repository_version_constants : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new comment on setupMsgHandlers still refers to mapping by envelope field number, but the code now uses m_msg_mapper->getMessageType(#msg) with no protocol ID; consider updating the description to reflect that message types are resolved by name under a single protocol to avoid future confusion.
- With the removal of protocol-specific IDs in SET_MSG_HANDLER, all handlers are now registered in a single namespace; if anonymous and authenticated messages can share names, you may want to document or enforce that these names remain unique to prevent subtle handler collisions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new comment on setupMsgHandlers still refers to mapping by envelope field number, but the code now uses m_msg_mapper->getMessageType(#msg) with no protocol ID; consider updating the description to reflect that message types are resolved by name under a single protocol to avoid future confusion.
- With the removal of protocol-specific IDs in SET_MSG_HANDLER, all handlers are now registered in a single namespace; if anonymous and authenticated messages can share names, you may want to document or enforce that these names remain unique to prevent subtle handler collisions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
JoshuaSBrown
merged commit 4f04fed
into
1843-DAPS-refactor-core-server-to-use-proto3
5 of 7 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
Description
How Has This Been Tested?
Artifacts (if appropriate):
Tasks
Summary by Sourcery
Refactor the repository server to use the new proto3 envelope and version constants for messaging and version reporting.
Enhancements: