RSDK-9175: DataSync API Go SDK #4562
Open
+1,031
−165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
purplenicole730
requested changes
Nov 21, 2024
| opts ...grpc.CallOption) (datapb.DataSyncService_StreamingDataCaptureUploadClient, error) | ||
| } | ||
|
|
||
| // DataCaptureUpload uploads the contents and metadata for tabular data. |
There was a problem hiding this comment.
In the inject folder, the comments of these fake functions tend to have the same/similar pattern:
i.e. // GetCurrentMonthUsage calls the injected GetCurrentMonthUsageFunc or the real version.
I'd follow the same thing here.
Comment on lines
+247
to
+254
| // BinaryOptions represents optional parameters for the BinaryDataCaptureUpload method. | ||
| type BinaryOptions struct { | ||
| Type DataType | ||
| FileName string | ||
| MethodParameters map[string]interface{} | ||
| Tags []string | ||
| DataRequestTimes [2]time.Time | ||
| } |
There was a problem hiding this comment.
I'm not sure what the standard is in Go, but should these optional parameter structs sit all the way at the top with the shadow types? Personally, it makes it hard for me to read the code so I actually prefer it being right above the function(s) that's using it if there's only one or two. Maybe someone else disagrees?
| partID string, | ||
| data []byte, | ||
| opts *FileUploadOptions, | ||
| ) (string, error) { |
There was a problem hiding this comment.
I see that this is an upload function, but what is it actually returning? It's not clear what this string value is supposed to represent.
There was a problem hiding this comment.
Ah, I see now from the private function you're calling it's the file ID, but it should be made clear in the comment.
| partID string, | ||
| filePath string, | ||
| opts *FileUploadOptions, | ||
| ) (string, error) { |
There was a problem hiding this comment.
See above comment about what the return is.
viambot
added
safe to test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the DataSync API wrappers to the Go SDK