Add TextIO and AvroIO withNumShards tests #92
Conversation
|
R: @dhalperi |
tgroh
force-pushed
the
text_avro_io_sharding_tests
branch
2 times, most recently
from
7b7b42e to
9a0dfec
Compare
|
tgroh
force-pushed
the
text_avro_io_sharding_tests
branch
from
9a0dfec to
13bad6b
Compare
|
I'll take this one from @dhalperi. R: @davorbonaci |
| // TODO: for Write only, test withSuffix, withNumShards, | ||
| @SuppressWarnings("deprecation") // using AvroCoder#createDatumReader for tests. | ||
| @Test | ||
| public void testAvroSinkShardedWrite() throws Exception { |
There was a problem hiding this comment.
This particular test (and probably others in this file) could be made runner-agnostic. That would make them much better.
However, this is an underlying problem here. Ok to ignore for now.
There was a problem hiding this comment.
Agreed; this would be best done by using the PipelineOptions.getTempLocation() for the location of the temporary files, and IOChannelUtils to either create input files or write to output files.
Filed BEAM-181
|
LGTM |
|
@tgroh if this is ready to merge R:me or otherwise bump back to Davor. |
tgroh
force-pushed
the
text_avro_io_sharding_tests
branch
from
c440722 to
f01ecbd
Compare
tgroh
force-pushed
the
text_avro_io_sharding_tests
branch
from
f01ecbd to
ea289fb
Compare
|
refactored to share code, should be ready to merge R: @dhalperi if you want to take a look or merge, |
| Bound<String> write = AvroIO.Write.to(outputFilePrefix) | ||
| .withSchema(String.class); | ||
| if (numShards > 1) { | ||
| write = write.withNumShards(numShards).withShardNameTemplate(ShardNameTemplate.INDEX_OF_MAX); |
There was a problem hiding this comment.
INDEX_OF_MAX is the default, right? Should be unnecessary.
There was a problem hiding this comment.
It is, but I was keeping symmetry.
Changed to use the write template while constructing expected files.
|
LGTM, optional comments. Ping me in the morning to merge. |
When we insert a partition record to the PartitionMetadata table, we will give a `TimestampConverter.MAX_MICROS` as default value. The default value should also be `TimestampConverter.MAX_MICROS + 1`.
Toward apache#92. Co-authored-by: Christopher Wilcox <crwilcox@google.com>
Note that the synthtool-generated `.coveragerc` (see apache#224) does *not* include all changes needed for 100% coverage: see: - googleapis/gapic-generator-python#171 - googleapis/gapic-generator-python#437 Closes apache#92. Closes apache#195.
4 participants
No description provided.