Skip to content

Feat/record proto #1091

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jun 15, 2025
Merged

Feat/record proto #1091

merged 7 commits into from
Jun 15, 2025

Conversation

SAKURA-CAT
Copy link
Member

@SAKURA-CAT SAKURA-CAT commented Jun 13, 2025 •
edited
Loading

Description

  • 定义实验、指标相关的record proto
  • 增加对yRange的校验测试，验证json序列化安全性
  • 增加workflow单元测试
  • 优化部分注释

for #1063

列的枚举定义

在column的proto中，我们定义了以下枚举：

  1. 列class
  2. 列type
  3. 组type

其中除了 列type为必传参数以外，其他为可选，这意味着只有列的枚举0值被设置为UNKNOWN：

  enum ColumnType {
    COL_UNKNOWN = 0;  // Unknown column type, used for error handling
    COL_FLOAT = 1;
    COL_IMAGE = 2;
    COL_AUDIO = 3;
    COL_TEXT = 4;
    COL_OBJECT3D = 5;
    COL_MOLECULE = 6;
    COL_ECHARTS = 7;
  }

其他的枚举中0为默认值（有效值），这样方便测试，减少繁琐的计算。

yRange解析测试

在go中要实现null和int共存是一件很麻烦的事情，我们通过使用指针实现了这一点，从而在json序列化时支持类似[null, int]的格式。
在这里需要直接访问protobuf结构体的值，以防Get函数在空指针时自动设置默认空值——但是在linter规则中这不允许，因此我们通过注释显式告诉linter：

yRange = []*int64{
        //nolint:protogetter  // We need pointers to represent left null during JSON serialization.
        record.GetChartYRange().Minval,
        //nolint:protogetter  // We need pointers to represent right null during JSON serialization.
        record.GetChartYRange().Maxval,
}

Record 包装message

在保存备份日志时所有message都保存在同一份文件中，因此需要一个统一的message包装器来打包、标识不同类型的message——我们称之为Record：

// Record is the base record type in swanlab.
// It is used to represent any kind of record in swanlab.
message Record{
  // Marks the type of the record.
  enum RecordType{
    RECORD_UNKNOWN = 0; // Unknown record type, used for error handling
    RECORD_SETUP = 1;
    RECORD_TEARDOWN = 2;
    RECORD_RUNTIME = 3;
    RECORD_COLUMN = 4;
    RECORD_MEDIA = 5;
    RECORD_SCALAR = 6;
    RECORD_LOG = 7;
  }
  RecordType message_type = 1;
  // The data of this record, it can be any type of data.
  google.protobuf.Any payload = 2;
}

@SAKURA-CAT SAKURA-CAT marked this pull request as ready for review June 13, 2025 06:37
@SAKURA-CAT SAKURA-CAT self-assigned this Jun 13, 2025
@SAKURA-CAT SAKURA-CAT added the 💪 enhancement New feature or request label Jun 13, 2025
Copilot
Copilot AI reviewed Jun 13, 2025
View reviewed changes
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces new proto definitions for metric, experiment, and project records, along with updated build scripts, go.mod dependency changes, and expanded tests to validate JSON serialization and yRange parsing.

  • Adds new .proto files and corresponding Python and Go generated code
  • Implements a parser in Go with unit tests for handling yRange conversion and enum validation
  • Updates build and CI configuration files and documentation for the core module

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.

Show a summary per file
File Description
swanlab/proto/record/v1/metric_pb2.pyi Added view of generated type definitions for metric records
swanlab/proto/record/v1/metric_pb2.py Generated Python file for metric proto changes
swanlab/proto/record/v1/metric.proto New proto definitions for runtime, column, media, scalar and log records
swanlab/proto/record/v1/experiment_pb2.pyi Added new experiment proto type definitions
swanlab/proto/record/v1/experiment_pb2.py Generated Python file for experiment proto changes
swanlab/proto/record/v1/experiment.proto New proto definitions for project and experiment records
script/build_proto.py Updated build instructions to include gRPC tools installation
core/pkg/pb/*.go Updated Go generated files (experiment, common, collector service)
core/internal/api/parse.go Introduced a Parser for proto-to-JSON conversion and yRange handling
core/internal/api/parse_test.go Added unit tests to validate yRange serialization and parser functionality
core/internal/api/api.go Minor documentation update in file header
core/go.mod Updated dependencies and added indirect modules
core/README.md Updated file template documentation
.github/workflows/test-core.yml Enhanced CI workflow with additional testing and module verification
Comments suppressed due to low confidence (2)

core/internal/api/parse.go:1

  • The file header title 'send.go' does not match the actual file name 'parse.go'. Consider updating the header to reflect the correct filename for clarity.
// @Title        send.go

core/internal/api/parse_test.go:70

  • [nitpick] Consider replacing 'string(rune(rand.Intn(1000)))' with 'strconv.Itoa(rand.Intn(1000))' for a clearer and more idiomatic conversion of an integer to a string.
record.ChartIndex:  "test-chart-index" + string(rune(rand.Intn(1000))),

Kaikaikaifang
Kaikaikaifang requested changes Jun 14, 2025
View reviewed changes
@SAKURA-CAT SAKURA-CAT force-pushed the feat/record-proto branch 4 times, most recently from 616e996 to 74b5b13 Compare June 14, 2025 17:13
@Kaikaikaifang Kaikaikaifang self-requested a review June 15, 2025 06:27
Kaikaikaifang
Kaikaikaifang approved these changes Jun 15, 2025
View reviewed changes
Copy link
Member

@Kaikaikaifang Kaikaikaifang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job! 👍

@SAKURA-CAT SAKURA-CAT merged commit dc7da17 into feature/core Jun 15, 2025
11 checks passed
@SAKURA-CAT SAKURA-CAT deleted the feat/record-proto branch June 15, 2025 06:33
SAKURA-CAT added a commit that referenced this pull request Jun 15, 2025
@SAKURA-CAT @Kaikaikaifang
Feature/core (#1100)
e3b3ae3 
* feat: init go project (#1064)

* fix: license use symlink (#1067)

* Feature/protobuf: hello-word (#1074)

* feat: init go project (#1064)

* feat: add proto and grpc

* feat: add proto build script

* fix: golangci lint

* feat: support auto or specify select port

* chore: license

* chore: add .golangci.yml

* fix: use log/slog instead of log

* chore: del core vendor

---------

Co-authored-by: Kang Li <79990647+SAKURA-CAT@users.noreply.github.com>

* feat: impl health checker and init data collector (#1077)

* feat: impl health checker and init data collector

* chore: change module name

---------

Co-authored-by: KAAANG <79990647+SAKURA-CAT@users.noreply.github.com>

* Feat/record proto and column parser (#1091)

Co-authored-by: kaikai <2837968358@qq.com>

* fix: import module and typo

---------

Co-authored-by: Kaikai <98205900+Kaikaikaifang@users.noreply.github.com>
Co-authored-by: kaikai <2837968358@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@Kaikaikaifang Kaikaikaifang Kaikaikaifang approved these changes

Assignees

@SAKURA-CAT SAKURA-CAT

Labels
💪 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SAKURA-CAT @Kaikaikaifang