Skip to content

Adds support for functions trigger API#583

Merged
andrewsomething merged 5 commits intodigitalocean:mainfrom
ddebarros:functions-trigger-api
Nov 29, 2022
Merged

Adds support for functions trigger API#583
andrewsomething merged 5 commits intodigitalocean:mainfrom
ddebarros:functions-trigger-api

Conversation

@ddebarros
Copy link
Copy Markdown
Contributor

@ddebarros ddebarros commented Nov 21, 2022

  • Adds support Functions trigger APIs (ListTriggers, GetTrigger, CreateTrigger, UpdateTrigger, DeleteTrigger)

https://docs.digitalocean.com/reference/api/api-reference/#operation/functions_list_triggers

Comment thread functions.go Outdated
@ddebarros ddebarros requested a review from piyush0101 November 21, 2022 19:29
piyush0101
piyush0101 previously approved these changes Nov 21, 2022
Copy link
Copy Markdown

@piyush0101 piyush0101 left a comment

Choose a reason for hiding this comment

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

looks good overall!

Comment thread functions.go
Comment thread functions.go
Comment thread functions.go Outdated
}

type FunctionsTriggerUpdateRequest struct {
IsEnabled bool `json:"is_enabled"`
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be a pointer? Do we support updating scheduled_details without sending is_enabled? Does the API differentiate between not sending is_enabled and sending is_enabled: false? Without making this a pointer, Go will default to setting this false if excluded.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks Andrew, yes the API does allow changing scheduled_details without changing is_enabled , I changed the FunctionsTriggerUpdateRequest.IsEnabled to be a *bool.

Copy link
Copy Markdown
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

@andrewsomething andrewsomething merged commit c7d7923 into digitalocean:main Nov 29, 2022
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