Fix Dataflow job file validation on Windows - #39708
Conversation
Signed-off-by: ulofiai <monsterking@tutamail.com>
|
Assigning reviewers: R: @jrmccluskey added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
jrmccluskey
left a comment
There was a problem hiding this comment.
Are there any existing test cases that are disabled on windows that we could use to validate the fix here? Or can we refactor a bit to make unit testing more viable?
| firstNonNull(options.getTemplateLocation(), options.getDataflowJobFile()); | ||
| checkArgument( | ||
| fileLocation.startsWith("/") || fileLocation.startsWith("gs://"), | ||
| fileLocation.startsWith("/") |
There was a problem hiding this comment.
Minor performance consideration. Considering the ordering:
fileLocation.startsWith("/") || fileLocation.startsWith("gs://") || new File(fileLocation).isAbsolute()
Nowadays most likely it would be a gcs path. Put new the last one avoids new an object each time it's executed
Fixes #18734.
Use
File.isAbsolute()when validatingdataflowJobFileandtemplateLocation. This accepts Windows drive-letter and UNC paths while preserving the existing Unix absolute-path andgs://checks.