Skip to content

feat(SDK+Proto): Revert semaphore/mutex fields in preparation for simplified design #12305

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
Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat(SDK+Proto): Revert semaphore/mutex fields in preparation for simplified design #12305

wants to merge 4 commits into from

Conversation

@DharmitD
Copy link
Member

@DharmitD DharmitD commented Sep 30, 2025 •
edited
Loading

Description of your changes:

This PR reverts the semaphore and mutex functionality introduced in PRs #11340 and #11384 to prepare for a simplified design approach that provides better user experience.

Reverted Changes

  • feat(api): Add SemaphoreKey and MutexName fields to proto (commit 28e5ba9d3)
  • feat(sdk): Add SemaphoreKey and MutexName fields to DSL (commit e997d426)

Background

After team discussion (referenced in this PR comment), we determined that the original approach requiring users to manually manage semaphore_key and mutex_name fields was too complex and required excessive Kubernetes knowledge.

Original approach issues:

  • Users needed to understand Kubernetes semaphores/mutexes
  • Required manual ConfigMap key management
  • Complex partially-managed ConfigMap state
  • Poor user experience for simple "limit concurrent runs" use case

New Simplified Design (Future PR)

The replacement design will introduce a single pipeline_run_parallelism integer field that:

  • Takes a simple integer value (e.g., pipeline_run_parallelism=3)
  • Automatically generates semaphore keys as <pipeline_name>/<pipeline_version>
  • Eliminates need for Kubernetes knowledge
  • Provides intuitive "max concurrent runs" semantics

Impact

  • Removes semaphore_key and mutex_name fields from PipelineConfig proto and SDK
  • Existing YAML pipelines using these fields will have them ignored (no errors)
  • No gRPC API impact (fields were only used for YAML/JSON parsing)

Testing

  • Protobuf regenerated and validated
  • All semaphore/mutex tests removed cleanly
  • Existing workspace functionality preserved
  • No breaking changes to core pipeline compilation

Checklist:

@DharmitD
fix: Remove semaphore/mutex fields from test proto objects
8f0feca 
Resolves CI failure where test code was still referencing removed
SemaphoreKey and MutexName fields in PipelineConfig struct literal.

Signed-off-by: ddalvi <[email protected]>
@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 assign chensun for approval. For more information see the Kubernetes Code Review Process.

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

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

@DharmitD
fix: Regenerate proto test fixtures after semaphore/mutex field removal
1fdec8e 
Updates platform_spec test data to match the new protobuf schema without
SemaphoreKey and MutexName fields. Resolves TestPlatformSpec failure.

Signed-off-by: ddalvi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@droctothorpe droctothorpe Awaiting requested review from droctothorpe

@VaniHaripriya VaniHaripriya Awaiting requested review from VaniHaripriya

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@DharmitD