Skip to content

feat(backend): Add plugin input/output fields to RecurringRun and Run protobufs#12919

Open
hbelmiro wants to merge 1 commit intokubeflow:mlflow-integrationfrom
hbelmiro:RHOAIENG-50329
Open

feat(backend): Add plugin input/output fields to RecurringRun and Run protobufs#12919
hbelmiro wants to merge 1 commit intokubeflow:mlflow-integrationfrom
hbelmiro:RHOAIENG-50329

Conversation

@hbelmiro
Copy link
Contributor

Description of your changes:
Adds a generic plugin metadata interface to Run and RecurringRun as specified in KEP-12862. This is the schema-only change -- no MLflow or plugin business logic is introduced.

  • Add MetadataValue, PluginOutput messages and plugins_input (field 19) / plugins_output (field 20) to Run
  • Add plugins_input (field 19) to RecurringRun
  • Propagate through Go models (*LargeText), storage layer (SQL NULL for absent), and API converters
  • Regenerate Go gRPC, swagger, and HTTP client code

Details

  • Model fields use *LargeText with nil = absent (SQL NULL), never empty string
  • Storage write paths use largeTextToNullableSQL; read paths use pointer assignment
  • GetRun/ListRuns/GetRecurringRun return plugins fields as stored, no external calls
  • Existing function signatures (toApiRun, toApiRecurringRun, etc.) are unchanged

Test plan

  • Unit tests for JSON serialization helpers (nil, empty, valid, malformed)
  • Converter tests for toModelRun, toApiRun, toModelJob, toApiRecurringRun (with data + nil)
  • Storage round-trip tests (create, get, update, list) for both runs and jobs
  • DB-level NULL assertions verifying absent fields produce SQL NULL, not empty string
  • Proto golden file tests updated

Checklist:

@google-oss-prow
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from hbelmiro. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@hbelmiro hbelmiro force-pushed the RHOAIENG-50329 branch 4 times, most recently from 59d5dd1 to ec7bebe Compare February 26, 2026 14:02
@hbelmiro
Copy link
Contributor Author

/retest

// Default. No special rendering.
UNSPECIFIED = 0;
// Render the value as a hyperlink.
URL = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need number, text types here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ContentType is a UI rendering hint, not a data type. The value itself is google.protobuf.Value which already carries type information (string, number, etc.). UNSPECIFIED means "render as-is" which covers plain text and numbers. URL is the only case that needs special rendering (hyperlink).
Maybe we should renamen it from ContentType to UIHint or something to avoid confusing. But that should be done in the KEP first.

PipelineSpec
Conditions string `gorm:"column:Conditions; not null;"`
Conditions string `gorm:"column:Conditions; not null;"`
PluginsInputString *LargeText `gorm:"column:PluginsInput; default:null;"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be my lack of knowledge, why are we adding pluginsinput to Job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Job is the Go model for RecurringRun.

@hbelmiro
Copy link
Contributor Author

/retest

… protobufs

Signed-off-by: Helber Belmiro <helber.belmiro@gmail.com>
@hbelmiro hbelmiro changed the base branch from master to mlflow-integration February 26, 2026 17:39
@hbelmiro hbelmiro marked this pull request as ready for review February 26, 2026 17:39
@google-oss-prow google-oss-prow bot requested a review from mprahl February 26, 2026 17:39
@hbelmiro hbelmiro marked this pull request as draft February 26, 2026 17:40
@hbelmiro hbelmiro marked this pull request as ready for review February 26, 2026 17:46
@google-oss-prow google-oss-prow bot requested a review from zazulam February 26, 2026 17:46
@VaniHaripriya
Copy link
Contributor

@hbelmiro Is AutoMigrate sufficient for adding plugins_input and plugins_output to run_details table, or should we include an explicit DB migration script to avoid schema discrepancies ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants