[BEAM-951] FileBasedSink: merge FileOperations into IOChannelFactory. #1329
Conversation
|
R: @tgroh |
|
needs some refactoring to resolve conflicts |
|
Ack. tgroh@ and davorbonaci@ can review in my absence, once you're ready. |
peihe
force-pushed
the
rm-file-operations
branch
2 times, most recently
from
fb1433d
to
6ef4c3c
Compare
|
PTAL PR synced |
| } | ||
| } | ||
|
|
||
| private void copyOne(String source, String destination) throws IOException { |
| private void copyOne(String source, String destination) throws IOException { | ||
| try { | ||
| // Copy the source file, replacing the existing destination. | ||
| // Paths.get(x) will not work on win cause of the ":" after the drive letter |
| StandardCopyOption.REPLACE_EXISTING); | ||
| } catch (NoSuchFileException e) { | ||
| LOG.debug("{} does not exist.", source); | ||
| // Suppress exception if file does not exist. |
There was a problem hiding this comment.
This seems as though it will be surprising.
There was a problem hiding this comment.
I agree.
However, improving the IOChannelFactory interface is out of the scope of the PR.
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
Seems @Experimental rather than @Deprecated
There was a problem hiding this comment.
Experimental only means no compatibility guarantees, and sdk/util already suggests that.
I think we also want to discourage people from using it, since we are deprecating it right now.
| } | ||
| } | ||
|
|
||
| private void copyOne(String source, String destination) throws IOException { |
| private void copyOne(String source, String destination) throws IOException { | ||
| try { | ||
| // Copy the source file, replacing the existing destination. | ||
| // Paths.get(x) will not work on win cause of the ":" after the drive letter |
| StandardCopyOption.REPLACE_EXISTING); | ||
| } catch (NoSuchFileException e) { | ||
| LOG.debug("{} does not exist.", source); | ||
| // Suppress exception if file does not exist. |
There was a problem hiding this comment.
I agree.
However, improving the IOChannelFactory interface is out of the scope of the PR.
| */ | ||
| @Deprecated |
There was a problem hiding this comment.
Experimental only means no compatibility guarantees, and sdk/util already suggests that.
I think we also want to discourage people from using it, since we are deprecating it right now.
peihe
force-pushed
the
rm-file-operations
branch
from
bbfe438
to
d280f10
Compare
|
rebased for Jenkins license issue |
|
LGTM @davorbonaci if you could take a look I'd appreciate it. |
|
LGTM |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.