Skip to content
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

feat: add s3 batch result code constants #302

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add s3 batch result code constants #302

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b5ac990
feat: add s3 batch result code constants
moofish32 Jun 28, 2020
ab4970b
chore: fix linting
moofish32 Jun 28, 2020
9e771cc
chore: remove breaking change proposal to s3 batch
moofish32 Oct 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions events/s3_batch_job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

package events

// S3BatchJobResultCode represents the result of an S3BatchJobTask (i.e.
// succeeded, permanent failure, or temmporary failure)
type S3BatchJobResultCode string

// S3BatchJobResultCode represents the result of an S3BatchJobTask (i.e.
// succeeded, permanent failure, or temmporary failure)
const (
S3BatchJobResultCodeSucceeded S3BatchJobResultCode = "Succeeded"
Copy link
Collaborator

Choose a reason for hiding this comment

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

downside of changing S3BatchJobResult.ResultCode type back to string, is that comparisons against this new type require a cast!

if inputEvent.Results[0].ResultCode == S3BatchJobResultCodeSucceeded {
  // ..
}

->

Invalid operation: inputEvent.Results[0].ResultCode == S3BatchJobResultCodeSucceeded (mismatched types string and S3BatchJobResultCode)

Instead, go back to having S3BatchJobResult being a S3BatchJobResultCode. It is useful for self documentation purposes. But also change the type to be a type alias eg: type S3BatchJobResultCoce = string - this will preserve backwards compatibility.

S3BatchJobResultCodeTemporaryFailure S3BatchJobResultCode = "TemporaryFailure"
S3BatchJobResultCodePermanentFailure S3BatchJobResultCode = "PermanentFailure"
)

// S3BatchJobEvent encapsulates the detail of a s3 batch job
type S3BatchJobEvent struct {
InvocationSchemaVersion string `json:"invocationSchemaVersion"`
Expand Down Expand Up @@ -33,7 +45,7 @@ type S3BatchJobResponse struct {

// S3BatchJobResult represents the result of a given task
type S3BatchJobResult struct {
TaskID string `json:"taskId"`
ResultCode string `json:"resultCode"`
ResultString string `json:"resultString"`
TaskID string `json:"taskId"`
ResultCode S3BatchJobResultCode `json:"resultCode"`
Copy link
Collaborator

@bmoffatt bmoffatt Oct 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think adding the type creates a breaking change. Assignment to a string will require a type cast

ex: https://play.golang.org/p/T5G1eKtbBor

I think leaving the type as string is fine

ResultString string `json:"resultString"`
}