[BEAM-2276] Cleanups to windowed DefaultFilenamePolicy#3232
[BEAM-2276] Cleanups to windowed DefaultFilenamePolicy#3232reuvenlax wants to merge 4 commits intoapache:masterfrom
Conversation
|
retest this please. |
1 similar comment
|
retest this please. |
|
Changes Unknown when pulling 6c69666 on reuvenlax:cleanup_filename_policy into ** on apache:master**. |
|
Thanks ! I'm taking a look. |
|
retest this please |
|
Changes Unknown when pulling 6c69666 on reuvenlax:cleanup_filename_policy into ** on apache:master**. |
|
Build finished. |
|
retest this please. |
|
retest this please |
|
--none-- |
1 similar comment
|
--none-- |
|
I don't see how the failure in org.apache.beam.sdk.io.jms.JmsIOTest.testAuthenticationWithBadPassword could be related to this PR. |
|
@reuvenlax if you take a look on the result in this PR, I don't think it's the latest build due to the issue we have currently between Jenkins and GitHub. Let me try and trigger a build by hand. |
|
retest this please |
|
Changes Unknown when pulling 6c69666 on reuvenlax:cleanup_filename_policy into ** on apache:master**. |
|
@jbonofre tests are now succeeding |
| ResourceId outputDirectory = LocalResources.fromString("/foo/bar", true /* isDirectory */); | ||
| FilenamePolicy policy = DefaultFilenamePolicy.constructUsingStandardParameters( | ||
| StaticValueProvider.of(outputDirectory), DefaultFilenamePolicy.DEFAULT_SHARD_TEMPLATE, ""); | ||
| StaticValueProvider.of(outputDirectory), DefaultFilenamePolicy |
| ResourceId outputDirectory = LocalResources.fromString("/foo", true /* isDirectory */); | ||
| FilenamePolicy policy = DefaultFilenamePolicy.constructUsingStandardParameters( | ||
| StaticValueProvider.of(outputDirectory), DefaultFilenamePolicy.DEFAULT_SHARD_TEMPLATE, ""); | ||
| StaticValueProvider.of(outputDirectory), DefaultFilenamePolicy |
| * | ||
| * <p>If provided, the suffix will be used; otherwise the files will have an empty suffix. | ||
| */ | ||
| public static DefaultFilenamePolicy constructUsingStandardParameters( |
There was a problem hiding this comment.
Some of these changes are backwards incompatible, and this class appears to be on the public API and not marked with @Experimental or @Internal.
There was a problem hiding this comment.
We have marked all of FileBasedSink experimental, and this is used by that. That should cover use of FilenamePolicy as well
|
Changes Unknown when pulling 79f95e9 on reuvenlax:cleanup_filename_policy into ** on apache:master**. |
5 participants
Make window strings more concise, and provide more information about the pane more concisely (most users care about whether a pane is the last one, and whether it is late).
Cleanup complicated code around filename templates - simply store two default templates, and pick the correct one based on whether windowed writes are enabled.
Remove log statements that would be extremely noisy in practice. Users may have valid reasons for not including W and P template variables - e.g. they may know they are in the global window and not want every file to contain GlobalWindow. This is the same reason we don't warn if S and N are missing from their template (they may be writing to a single shard).
R: @jbonofre