Skip to content

*: process actions returned by handler#57

Closed
hasbro17 wants to merge 1 commit intooperator-framework:masterfrom
hasbro17:haseeb/add-action
Closed

*: process actions returned by handler#57
hasbro17 wants to merge 1 commit intooperator-framework:masterfrom
hasbro17:haseeb/add-action

Conversation

@hasbro17
Copy link
Copy Markdown
Contributor

@hasbro17 hasbro17 commented Feb 23, 2018

The informer's sync will now process the actions returned by Handle().
The two supported actions are:

  • kube-apply: Creates or Updates the provided object
  • kube-delete: Deletes the resource specified by the object

@fanminshi fanminshi mentioned this pull request Feb 24, 2018
21 tasks
Comment thread pkg/sdk/action/action.go
}

const (
// Function names for supported functions
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

have you consider using enum for function name?

@hongchaodeng
Copy link
Copy Markdown
Contributor

@hasbro17
Can you separate the PR into two for each func?

// For actions done before the failed action, there is no rollback.
type Handler interface {
Handle(sdkTypes.Context, sdkTypes.Event) []sdkTypes.Action
Handle(sdkTypes.Context, sdkTypes.Event) []sdkAction.Action
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

'Action' should be in sdk/types.
Only the action implementations go into sdk/action.

Comment thread pkg/sdk/action/action.go
)

// FuncName defines the name of the function of an Action.
type FuncName string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Put FuncName to sdk/types too?

@hasbro17
Copy link
Copy Markdown
Contributor Author

Going to split the PR into two for kube-apply and kube-delete. The PR size won't change by much since kube-delete is just slightly different from kube-apply.
So feel free to keep reviewing the PR in the meanwhile.

Comment thread pkg/sdk/action/action.go
}

// Update it if it already exists
// TODO: Should resourceVersion conflict be handled differently
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe brainstorm about all the potential failure cases and list them in TODO?

@hasbro17 hasbro17 mentioned this pull request Feb 26, 2018
@hasbro17
Copy link
Copy Markdown
Contributor Author

Closing in favour of #58

@hasbro17 hasbro17 closed this Feb 26, 2018
m1kola pushed a commit to m1kola/operator-sdk that referenced this pull request Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants