-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Implement Project upload with V4 and selectable items. #48
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just noted a few smalll things that I came across
time.Sleep(1 * time.Second) | ||
tries++ | ||
} else { | ||
return errors.New("import task did not complete successfully. Status is \"" + importStatus.Status + "\". Please check the FME Flow web interface for the status of the import task") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return errors.New("import task did not complete successfully. Status is \"" + importStatus.Status + "\". Please check the FME Flow web interface for the status of the import task") | |
return errors.New("Import task did not complete successfully. Status is \"" + importStatus.Status + "\". Please check the FME Flow web interface for the status of the import task") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go is actually picky about this and shouldn't start with a capital letter: https://go.dev/wiki/CodeReviewComments#error-strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake!
if importStatus.Status == "imported" { | ||
finished = true | ||
} else if importStatus.Status != "importing" { | ||
return errors.New("import task did not complete successfully. Please check the FME Flow web interface for the status of the import task") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return errors.New("import task did not complete successfully. Please check the FME Flow web interface for the status of the import task") | |
return errors.New("Import task did not complete successfully. Please check the FME Flow web interface for the status of the import task") |
// validate that the selected items are the correct format | ||
match, _ := regexp.MatchString(`^([^:,]+:[^:,]+,)*([^:,]+:[^:,]+)$`, f.selectedItems) | ||
if !match { | ||
return errors.New("invalid selected items. Must be a comma separated list of item ids and types. e.g. item1:itemtype1,item2:itemtype2") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return errors.New("invalid selected items. Must be a comma separated list of item ids and types. e.g. item1:itemtype1,item2:itemtype2") | |
return errors.New("Invalid selected items. Must be a comma separated list of item ids and types. e.g. item1:itemtype1,item2:itemtype2") |
} | ||
} | ||
if !found { | ||
return errors.New("selected item " + id + " (" + itemType + ") is not in the list of selectable items") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return errors.New("selected item " + id + " (" + itemType + ") is not in the list of selectable items") | |
return errors.New("Selected item " + id + " (" + itemType + ") is not in the list of selectable items") |
Thanks! I have made the changes suggested, with the exception of the error message capitalization: https://go.dev/wiki/CodeReviewComments#error-strings |
The project upload was previously using the V3 API. This pull request will use the V4 API on builds that support it. This also allows for selectable items to be excluded if desired.