[BEAM-50] Fix BigQuery.Write tempFilePrefix concatenation #192
Conversation
|
R: @dhalperi |
|
Sorry for the delay. I was hoping to get @lukecwik 's opinion on this one. |
peihe
force-pushed
the
fix-path-resolve
branch
3 times, most recently
from
8e44737 to
394ca71
Compare
| } | ||
| String jobIdToken = UUID.randomUUID().toString(); | ||
| String tempFilePrefix = options.getTempLocation() + "/BigQuerySinkTemp/" + jobIdToken; | ||
| String tempLocation = options.getTempLocation(); |
There was a problem hiding this comment.
You should rely on IOChannelUtils / IOChannelFactory and not call out to paths directly.
For example:
IOChannelFactory factory = IOChannelUtils.getFactory(path);
String tempFilePrefix = factory.resolve(factory.resolve(tempLocation, "BigQuerySinkTemp"), jobIdToken);
checkArgument(testBigQueryService == null || factory instanceof GCSIOChannelFactory, ....);
Other than this comment I think the code is fine.
There was a problem hiding this comment.
Done, thanks!
Removed the check of gcs path, since it is done in validate().
In there (line 993), I am using GcsPath directly and expect it fails for non gcs path.
I am not checking the GCSIOChannelFactory, because I think there might be other implementations.
There was a problem hiding this comment.
LGTM
Dan, any additional comments?
peihe
force-pushed
the
fix-path-resolve
branch
2 times, most recently
from
e91dd9d to
bc49128
Compare
Apply ModelEnforcement in the InProcessPipelineRunner
3 participants
No description provided.