-
Notifications
You must be signed in to change notification settings - Fork 16
feat: add protobuf3 replacement files. #1836
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Conversation
Reviewer's GuideIntroduces a new proto3-based common protocol module by adding a dedicated CMake target that compiles all proto3 definitions into a C++ library, along with the full set of protobuf3 message, enum, and request/response definition files organized under anon, auth, messages, and enums namespaces. Class diagram for proto3 protocol module organizationclassDiagram
class common_proto3_common {
}
class enums {
<<namespace>>
access_token_type
dependency_dir
dependency_type
encryption
error_code
execution_method
note_state
note_type
project_role
search_mode
service_status
sort_option
task_command
task_status
task_type
}
class messages {
<<namespace>>
acl_rule
alloc_data
alloc_stats_data
coll_data
dependency_data
dependency_spec_data
group_data
listing_data
note_comment
note_data
path_data
project_data
record_data
record_data_location
record_data_size
repo_data
repo_record_data_locations
schema_data
tag_data
task_data
topic_data
user_data
}
class anon {
<<namespace>>
ack_reply
auth_status_reply
authenticate_by_password_request
authenticate_by_token_request
daily_message_reply
daily_message_request
get_auth_status_request
nack_reply
version_reply
version_request
}
class auth {
<<namespace>>
acl_data_reply
acl_shared_list_items_request
acl_shared_list_request
acl_update_request
acl_view_request
check_perms_reply
check_perms_request
coll_create_request
coll_data_reply
coll_delete_request
coll_get_offset_reply
coll_get_offset_request
coll_get_parents_request
coll_list_published_request
coll_move_request
coll_path_reply
coll_read_request
coll_update_request
coll_view_request
coll_write_request
data_delete_request
data_get_reply
data_get_request
data_path_reply
data_path_request
data_put_reply
data_put_request
generate_credentials_reply
generate_credentials_request
get_perms_reply
get_perms_request
group_create_request
group_data_reply
group_delete_request
group_list_request
group_update_request
group_view_request
listing_reply
metadata_validate_reply
metadata_validate_request
note_comment_edit_request
note_create_request
note_data_reply
note_list_by_subject_request
note_update_request
note_view_request
project_create_request
project_data_reply
project_delete_request
project_get_role_reply
project_get_role_request
project_list_request
project_search_request
project_update_request
project_view_request
query_create_request
query_data_reply
query_delete_request
query_exec_request
query_list_request
query_update_request
query_view_request
record_alloc_change_reply
record_alloc_change_request
record_create_batch_request
record_create_request
record_data_reply
record_delete_request
record_export_reply
record_export_request
record_get_dependency_graph_request
record_list_by_alloc_request
record_lock_request
record_owner_change_reply
record_owner_change_request
record_update_batch_request
record_update_request
record_view_request
repo_allocation_create_request
repo_allocation_create_response
repo_allocation_delete_request
repo_allocation_set_default_request
repo_allocation_set_request
repo_allocation_stats_reply
repo_allocation_stats_request
repo_allocations_reply
repo_authz_request
repo_calc_size_reply
repo_calc_size_request
repo_create_request
repo_data_delete_request
repo_data_get_size_request
repo_data_reply
repo_data_size_reply
repo_delete_request
repo_list_allocations_request
repo_list_object_allocations_request
repo_list_request
repo_list_subject_allocations_request
repo_path_create_request
repo_path_delete_request
repo_update_request
repo_view_allocation_request
repo_view_request
revoke_credentials_request
schema_create_request
schema_data_reply
schema_delete_request
schema_revise_request
schema_search_request
schema_update_request
schema_view_request
search_request
tag_data_reply
tag_list_by_count_request
tag_search_request
task_data_reply
task_list_request
task_view_request
topic_data_reply
topic_list_topics_request
topic_search_request
topic_view_request
user_access_token_reply
user_create_request
user_data_reply
user_find_by_name_uid_request
user_find_by_uuids_request
user_get_access_token_request
user_get_recent_ep_reply
user_get_recent_ep_request
user_list_all_request
user_list_collab_request
user_set_access_token_request
user_set_recent_ep_request
user_update_request
user_view_request
}
class envelope {
<<message>>
envelope
}
common_proto3_common o-- enums
common_proto3_common o-- messages
common_proto3_common o-- anon
common_proto3_common o-- auth
common_proto3_common o-- envelope
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey - I've left some high level feedback:
- Using
file(GLOB_RECURSE ...)forProtoFilescan make builds non-deterministic when proto files are added/removed without touching CMake; consider listing the proto files explicitly or at least constraining the glob by directory/extension pattern to what you expect long term. - The
target_include_directories(datafed-protobuf INTERFACE ${PROJECT_BINARY_DIR}/common/proto)path looks like it still points to the old proto location; double-check that consumers of this target see the correct include directory for the new proto3-generated headers (likely${PROJECT_BINARY_DIR}/common/proto3/commonor${CMAKE_CURRENT_BINARY_DIR}).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `file(GLOB_RECURSE ...)` for `ProtoFiles` can make builds non-deterministic when proto files are added/removed without touching CMake; consider listing the proto files explicitly or at least constraining the glob by directory/extension pattern to what you expect long term.
- The `target_include_directories(datafed-protobuf INTERFACE ${PROJECT_BINARY_DIR}/common/proto)` path looks like it still points to the old proto location; double-check that consumers of this target see the correct include directory for the new proto3-generated headers (likely `${PROJECT_BINARY_DIR}/common/proto3/common` or `${CMAKE_CURRENT_BINARY_DIR}`).Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…hat was there before, update INCLUDE directive in CMakeLists.txt from proto to proto3
Ticket
#1835
Description
Tasks
Summary by Sourcery
Introduce a new proto3-based common protocol module and library for DataFed.
New Features: