feat: add manual trigger for lakefs actions #9338

skshetry · 2025-07-23T10:53:04Z

Overview

This PR implements a new manual trigger event type for lakeFS actions, allowing users to trigger actions on-demand via API calls. The manual trigger executes actions asynchronously in the background and returns immediately with a 202 Accepted status.

Changes

Core Implementation

Manual Trigger Event Type: Added manual-trigger event type to the actions system
Async Execution: Manual triggers run in the background using asyncRun for non-blocking execution
API Endpoint: New POST /repositories/{repository}/refs/{ref}/actions/{action}/triggers endpoint
Response Status: Returns 202 Accepted for async operations with no response body

Usage

Action Definition

name: manual-action
on:
  manual-trigger:
hooks:
  - id: manual-hook
    type: lua
    properties:
      script: |
        print("Manual trigger executed!")

API Call

curl -X POST \
  "http://localhost:8000/api/v1/repositories/my-repo/refs/main/actions/manual-action/triggers" \
  -H "Authorization: Bearer <token>"

Response

HTTP/1.1 202 Accepted

Testing

Not yet.

Related Issues

Closes #5778

skshetry · 2025-07-23T13:20:08Z

pkg/actions/service.go

+	repoRecord := &graveler.RepositoryRecord{
+		RepositoryID: graveler.RepositoryID(repository.Name),
+		Repository: &graveler.Repository{
+			StorageID:        graveler.StorageID(repository.StorageID),
+			StorageNamespace: graveler.StorageNamespace(repository.StorageNamespace),
+			CreationDate:     repository.CreationDate,
+			DefaultBranchID:  graveler.BranchID(repository.DefaultBranch),
+			State:            graveler.RepositoryState_ACTIVE,
+			InstanceUID:      "", // Not available from catalog API
+			ReadOnly:         repository.ReadOnly,
+		},
+	}


There must be a better way to get RepositoryRecord that I could not find. I asked cursor and failed, got this response instead:

This pattern is used throughout the codebase when converting between the catalog API layer and the internal graveler layer.

This is actually how we do it. This is kinda a pattern - the different layers use different data object types. The most common use of this pattern outside of lakeFS is probably in DTOs.
@nopcoder for more context on why we do this.

yeah cursor is wrong :)

skshetry · 2025-07-23T13:21:25Z

pkg/api/controller.go

+	}
+
+	// For async execution, just return 202 Accepted
+	writeResponse(w, r, http.StatusAccepted, nil)


Should this return something? If not, should this return 204?

"202 Accepted" is actually a good choice! MDN says about 202:

indicates that a request has been accepted for processing, but processing has not been completed or may not have started

GitHub does 204 on workflow_dispatch, and does not return a body.

https://docs.github.com/en/rest/actions/workflows?apiVersion=2022-11-28#create-a-workflow-dispatch-event

So, I was trying to imitate that. If we return a body, 202 makes sense.

pkg/actions/action.go

arielshaqed

I would expect manually triggering an action to be able to trigger a particular action, rather than all action configured with a manual trigger. So it is rather different from the other "hooks" - clearly a post-commit hook should run all hooks configured for post commit.

I am requesting changes chiefly because I expected a different feature than this defines. I might very well be (very) wrong - and then we should pretty much ignore my review.

arielshaqed · 2025-07-23T13:20:57Z

api/swagger.yml

@@ -5597,6 +5597,37 @@ paths:
        default:
          $ref: "#/components/responses/ServerError"

+  /repositories/{repository}/refs/{ref}/actions/trigger:


Not sure I understand the feature: which action will this run?

arielshaqed · 2025-07-23T13:21:47Z

pkg/actions/action.go

+		graveler.EventTypePreCherryPick,
+		graveler.EventTypePostCherryPick,


Is this an unrelated bugfix?

cc @nopcoder as I see you implemented this in #9263.

it looks like a bug fix for my pr, can take it as a different pr.
and I need to think of a better way to validate next time I'll not miss this one.

arielshaqed · 2025-07-23T13:23:39Z

pkg/actions/service.go

+	repoRecord := &graveler.RepositoryRecord{
+		RepositoryID: graveler.RepositoryID(repository.Name),
+		Repository: &graveler.Repository{
+			StorageID:        graveler.StorageID(repository.StorageID),
+			StorageNamespace: graveler.StorageNamespace(repository.StorageNamespace),
+			CreationDate:     repository.CreationDate,
+			DefaultBranchID:  graveler.BranchID(repository.DefaultBranch),
+			State:            graveler.RepositoryState_ACTIVE,
+			InstanceUID:      "", // Not available from catalog API
+			ReadOnly:         repository.ReadOnly,
+		},
+	}


This is actually how we do it. This is kinda a pattern - the different layers use different data object types. The most common use of this pattern outside of lakeFS is probably in DTOs.
@nopcoder for more context on why we do this.

arielshaqed · 2025-07-23T13:26:35Z

pkg/api/controller.go

+	_, err = c.Actions.TriggerAction(ctx, repo, refParam)
+	if c.handleAPIError(ctx, w, r, err) {
+		return
+	}
+
+	// For async execution, just return 202 Accepted
+	writeResponse(w, r, http.StatusAccepted, nil)


I think I might kind-of almost agree with copilot here. But of course we would need to define the body payload in swagger.

itaiad200

Simple and solid implementation. I think I would choose a slightly different approach with the triggering, i.e. choose the specific action to be triggered. Lacks tests and documentation, but there's a limit to what can be achieved in a day. Good job!

api/swagger.yml

itaiad200 · 2025-07-23T13:35:22Z

pkg/actions/service.go

@@ -210,6 +211,7 @@ type Service interface {
 	GetTaskResult(ctx context.Context, repositoryID string, runID string, hookRunID string) (*TaskResult, error)
 	ListRunResults(ctx context.Context, repositoryID string, branchID, commitID string, after string) (RunResultIterator, error)
 	ListRunTaskResults(ctx context.Context, repositoryID string, runID string, after string) (TaskResultIterator, error)
+	TriggerAction(ctx context.Context, repository *catalog.Repository, ref string) (string, error)


isn't it better to leverage the existing Run and tweak the HookRecord?

Thanks for the idea. I have changed to do as you suggested.

Run() runs synchronously, so I adjusted that to use asyncRun().

itaiad200 · 2025-07-23T13:36:15Z

pkg/actions/service.go

+	repoRecord := &graveler.RepositoryRecord{
+		RepositoryID: graveler.RepositoryID(repository.Name),
+		Repository: &graveler.Repository{
+			StorageID:        graveler.StorageID(repository.StorageID),
+			StorageNamespace: graveler.StorageNamespace(repository.StorageNamespace),
+			CreationDate:     repository.CreationDate,
+			DefaultBranchID:  graveler.BranchID(repository.DefaultBranch),
+			State:            graveler.RepositoryState_ACTIVE,
+			InstanceUID:      "", // Not available from catalog API
+			ReadOnly:         repository.ReadOnly,
+		},
+	}


yeah cursor is wrong :)

itaiad200 · 2025-07-23T13:37:31Z

pkg/actions/service.go

+		SourceRef:  graveler.Ref(ref),
+		Repository: repoRecord,
+	}
+	s.asyncRun(ctx, record)


So we're basically running all actions that are on that ref and registered to manual trigger event trigger, right?
This is slightly different than Github API where you pass the action you want to trigger.

I have fixed this in the new iteration.

arielshaqed

It's good code, but I'm worried whether it's the desired capability. I would like something that more closely mirrors what GitHub does. On GitHub, I get to run manually some particular action, and even pick parameters such as "branch". I don't need to run all actions for a branch on a repo.
If we do it this way, for instance, we might be painting ourselves into a corner. When we add a hook that is triggered by other parameters (think of a post-upload hook perhaps), it will be very weird to trigger it like this.

arielshaqed · 2025-07-23T14:44:09Z

pkg/api/controller.go

+	}
+
+	// For async execution, just return 202 Accepted
+	writeResponse(w, r, http.StatusAccepted, nil)


"202 Accepted" is actually a good choice! MDN says about 202:

indicates that a request has been accepted for processing, but processing has not been completed or may not have started

skshetry · 2025-07-23T14:51:08Z

It's good code, but I'm worried whether it's the desired capability. I would like something that more closely mirrors what GitHub does. On GitHub, I get to run manually some particular action, and even pick parameters such as "branch". I don't need to run all actions for a branch on a repo. If we do it this way, for instance, we might be painting ourselves into a corner. When we add a hook that is triggered by other parameters (think of a post-upload hook perhaps), it will be very weird to trigger it like this.

@arielshaqed, that was what cursor generated in 4c3290f, but I asked it to simplify as it was not clear to me what was expected.

I can revert but need to figure out how to run those in async way (Cursor's code ran tasks synchronously).

Copilot

Pull Request Overview

This PR implements a manual trigger functionality for lakeFS actions, allowing users to trigger actions on-demand via API calls rather than only through repository events. The feature enables async execution with immediate 202 Accepted responses.

Key changes include:

Added manual-trigger event type to the actions system with async execution capabilities
New API endpoint for triggering actions manually with proper authorization
Client SDK updates across multiple languages (Rust, Python, Java) with comprehensive documentation

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/permissions/actions.go	Adds new `TriggerActionsAction` permission constant for authorization
pkg/graveler/hooks_handler.go	Defines `EventTypeManualTrigger` event type and `ActionName` field in HookRecord
pkg/actions/service.go	Implements core `TriggerAction` method with action validation and async execution
pkg/api/controller.go	Adds HTTP endpoint handler with authorization and error handling
api/swagger.yml	Defines OpenAPI specification for the new trigger endpoint
clients/*	Generated client SDK code and documentation updates

docs/src/assets/js/swagger.yml

Copilot · 2025-07-23T16:57:31Z

pkg/actions/service.go

+			CreationDate:     repository.CreationDate,
+			DefaultBranchID:  graveler.BranchID(repository.DefaultBranch),
+			State:            graveler.RepositoryState_ACTIVE,
+			InstanceUID:      "", // Not available from catalog API


Setting InstanceUID to an empty string with a comment indicating it's not available could lead to issues if this field is required downstream. Consider either fetching this value properly or documenting the implications of leaving it empty.

Copilot · 2025-07-23T16:57:31Z

pkg/actions/service.go

+		EventType:  graveler.EventTypeManualTrigger,
+		Repository: repoRecord,
+		SourceRef:  graveler.Ref(ref),
+		BranchID:   graveler.BranchID(ref), // For manual triggers, we use the ref as branch


Directly casting ref to BranchID assumes the ref is always a branch, but refs can also be tags or commit hashes. This could lead to incorrect behavior when triggering actions on non-branch refs. Consider validating that the ref is actually a branch or handling different ref types appropriately.

Copilot · 2025-07-23T16:57:31Z

pkg/actions/service.go

+		EventType: record.EventType,
+		BranchID:  record.BranchID,
+	}
+	_, err := s.loadMatchedActions(ctx, record, spec)


The code calls loadMatchedActions twice - once for validation and again in asyncRun. This duplicates the action loading and filtering logic. Consider refactoring to load actions once and pass them to asyncRun, or cache the result to avoid redundant processing.

skshetry changed the title ~~initial changes~~ add manual trigger Jul 23, 2025

skshetry marked this pull request as ready for review July 23, 2025 13:03

skshetry changed the title ~~add manual trigger~~ feat: add manual trigger for lakefs actions Jul 23, 2025

arielshaqed requested review from arielshaqed and Copilot July 23, 2025 13:18

This comment was marked as outdated.

Sign in to view

skshetry commented Jul 23, 2025

View reviewed changes

pkg/actions/action.go Show resolved Hide resolved

arielshaqed requested changes Jul 23, 2025

View reviewed changes

itaiad200 requested changes Jul 23, 2025

View reviewed changes

arielshaqed reviewed Jul 23, 2025

View reviewed changes

skshetry force-pushed the feat/manual-trigger branch from 01972ae to f7cc263 Compare July 23, 2025 16:44

skshetry requested a review from Copilot July 23, 2025 16:56

Copilot AI reviewed Jul 23, 2025

View reviewed changes

skshetry added 2 commits July 24, 2025 09:17

initial changes

50e25cc

autogenerated changes

4094bd1

skshetry force-pushed the feat/manual-trigger branch from f7cc263 to 4094bd1 Compare July 24, 2025 03:32

		graveler.EventTypePreCherryPick,
		graveler.EventTypePostCherryPick,

feat: add manual trigger for lakefs actions #9338

Are you sure you want to change the base?

feat: add manual trigger for lakefs actions #9338

Uh oh!

Conversation

skshetry commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Core Implementation

Usage

Action Definition

API Call

Response

Testing

Related Issues

Uh oh!

This comment was marked as outdated.

Uh oh!

skshetry Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skshetry Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skshetry Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arielshaqed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itaiad200 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arielshaqed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skshetry commented Jul 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

skshetry commented Jul 23, 2025 •

edited

Loading

skshetry Jul 23, 2025 •

edited

Loading

skshetry Jul 23, 2025 •

edited

Loading

skshetry Jul 23, 2025 •

edited

Loading