Skip to content

Commit 1192632

Browse files
pratyakshsharmayingsu00
authored andcommitted
Read dataDirectory from system/env variables
Refactor IcebergQueryRunner and HiveQueryRunner to read the data directory from system variables rather than program arguments. This is done to ensure these runners are in sync with HiveExternalWorkerQueryRunner.
1 parent 66f34de commit 1192632

File tree

4 files changed

+61
-57
lines changed

4 files changed

+61
-57
lines changed

presto-hive/src/test/java/com/facebook/presto/hive/HiveQueryRunner.java

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import static com.facebook.presto.SystemSessionProperties.GROUPED_EXECUTION;
6161
import static com.facebook.presto.SystemSessionProperties.HASH_PARTITION_COUNT;
6262
import static com.facebook.presto.SystemSessionProperties.PARTITIONING_PROVIDER_CATALOG;
63+
import static com.facebook.presto.hive.HiveTestUtils.getDataDirectoryPath;
6364
import static com.facebook.presto.spi.security.SelectedRole.Type.ROLE;
6465
import static com.facebook.presto.testing.TestingSession.testSessionBuilder;
6566
import static com.facebook.presto.tests.QueryAssertions.copyTables;
@@ -441,42 +442,19 @@ public static void main(String[] args)
441442
throws Exception
442443
{
443444
// You need to add "--user user" to your CLI for your queries to work
444-
Logging.initialize();
445-
446-
Optional<Path> dataDirectory = Optional.empty();
445+
setupLogging();
446+
Optional<Path> dataDirectory;
447447
if (args.length > 0) {
448448
if (args.length != 1) {
449449
log.error("usage: HiveQueryRunner [dataDirectory]\n");
450450
log.error(" [dataDirectory] is a local directory under which you want the hive_data directory to be created.]\n");
451451
System.exit(1);
452452
}
453-
454-
File dataDirectoryFile = new File(args[0]);
455-
if (dataDirectoryFile.exists()) {
456-
if (!dataDirectoryFile.isDirectory()) {
457-
log.error("Error: " + dataDirectoryFile.getAbsolutePath() + " is not a directory.");
458-
System.exit(1);
459-
}
460-
else if (!dataDirectoryFile.canRead() || !dataDirectoryFile.canWrite()) {
461-
log.error("Error: " + dataDirectoryFile.getAbsolutePath() + " is not readable/writable.");
462-
System.exit(1);
463-
}
464-
}
465-
else {
466-
// For user supplied path like [path_exists_but_is_not_readable_or_writable]/[paths_do_not_exist], the hadoop file system won't
467-
// be able to create directory for it. e.g. "/aaa/bbb" is not creatable because path "/" is not writable.
468-
while (!dataDirectoryFile.exists()) {
469-
dataDirectoryFile = dataDirectoryFile.getParentFile();
470-
}
471-
if (!dataDirectoryFile.canRead() || !dataDirectoryFile.canWrite()) {
472-
log.error("Error: The ancestor directory " + dataDirectoryFile.getAbsolutePath() + " is not readable/writable.");
473-
System.exit(1);
474-
}
475-
}
476-
477-
dataDirectory = Optional.of(dataDirectoryFile.toPath());
453+
dataDirectory = getDataDirectoryPath(Optional.of(args[0]));
454+
}
455+
else {
456+
dataDirectory = getDataDirectoryPath(Optional.empty());
478457
}
479-
480458
DistributedQueryRunner queryRunner = createQueryRunner(TpchTable.getTables(), getAllTpcdsTableNames(), ImmutableMap.of("http-server.http.port", "8080"), dataDirectory);
481459

482460
try {

presto-hive/src/test/java/com/facebook/presto/hive/HiveTestUtils.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import com.facebook.airlift.json.JsonCodec;
1717
import com.facebook.airlift.json.smile.SmileCodec;
18+
import com.facebook.airlift.log.Logger;
1819
import com.facebook.presto.PagesIndexPageSorter;
1920
import com.facebook.presto.cache.CacheConfig;
2021
import com.facebook.presto.common.block.BlockEncodingManager;
@@ -79,7 +80,9 @@
7980
import com.google.common.collect.ImmutableSet;
8081
import io.airlift.slice.Slice;
8182

83+
import java.io.File;
8284
import java.math.BigDecimal;
85+
import java.nio.file.Path;
8386
import java.util.ArrayList;
8487
import java.util.List;
8588
import java.util.Optional;
@@ -94,6 +97,8 @@
9497

9598
public final class HiveTestUtils
9699
{
100+
private static final Logger log = Logger.get(HiveTestUtils.class);
101+
97102
private HiveTestUtils()
98103
{
99104
}
@@ -312,6 +317,42 @@ public static Optional<String> getProperty(String name)
312317
}
313318
}
314319

320+
public static Optional<Path> getDataDirectoryPath(Optional<String> suppliedDataDirectoryPath)
321+
{
322+
Optional<Path> dataDirectory = Optional.empty();
323+
if (!suppliedDataDirectoryPath.isPresent()) {
324+
//in case the path is not supplied as program argument, read it from env variable.
325+
suppliedDataDirectoryPath = getProperty("DATA_DIR");
326+
}
327+
if (suppliedDataDirectoryPath.isPresent()) {
328+
File dataDirectoryFile = new File(suppliedDataDirectoryPath.get());
329+
if (dataDirectoryFile.exists()) {
330+
if (!dataDirectoryFile.isDirectory()) {
331+
log.error("Error: " + dataDirectoryFile.getAbsolutePath() + " is not a directory.");
332+
System.exit(1);
333+
}
334+
else if (!dataDirectoryFile.canRead() || !dataDirectoryFile.canWrite()) {
335+
log.error("Error: " + dataDirectoryFile.getAbsolutePath() + " is not readable/writable.");
336+
System.exit(1);
337+
}
338+
}
339+
else {
340+
// For user supplied path like [path_exists_but_is_not_readable_or_writable]/[paths_do_not_exist], the hadoop file system won't
341+
// be able to create directory for it. e.g. "/aaa/bbb" is not creatable because path "/" is not writable.
342+
while (!dataDirectoryFile.exists()) {
343+
dataDirectoryFile = dataDirectoryFile.getParentFile();
344+
}
345+
if (!dataDirectoryFile.canRead() || !dataDirectoryFile.canWrite()) {
346+
log.error("Error: The ancestor directory " + dataDirectoryFile.getAbsolutePath() + " is not readable/writable.");
347+
System.exit(1);
348+
}
349+
}
350+
351+
dataDirectory = Optional.of(dataDirectoryFile.toPath());
352+
}
353+
return dataDirectory;
354+
}
355+
315356
public static List<PropertyMetadata<?>> getAllSessionProperties(HiveClientConfig hiveClientConfig, HiveCommonClientConfig hiveCommonClientConfig)
316357
{
317358
return getAllSessionProperties(hiveClientConfig, new ParquetFileWriterConfig(), hiveCommonClientConfig);

presto-iceberg/pom.xml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@
9292
<artifactId>animal-sniffer-annotations</artifactId>
9393
</exclusion>
9494
</exclusions>
95-
<scope>compile</scope>
9695
</dependency>
9796

9897
<dependency>
@@ -538,6 +537,13 @@
538537
<scope>test</scope>
539538
</dependency>
540539

540+
<dependency>
541+
<groupId>com.facebook.presto</groupId>
542+
<artifactId>presto-hive</artifactId>
543+
<type>test-jar</type>
544+
<scope>test</scope>
545+
</dependency>
546+
541547
<dependency>
542548
<groupId>com.facebook.presto</groupId>
543549
<artifactId>presto-tests</artifactId>

presto-iceberg/src/test/java/com/facebook/presto/iceberg/IcebergQueryRunner.java

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import com.google.common.collect.ImmutableSet;
3939
import io.airlift.tpch.TpchTable;
4040

41-
import java.io.File;
4241
import java.io.IOException;
4342
import java.net.URI;
4443
import java.nio.file.Files;
@@ -51,6 +50,7 @@
5150

5251
import static com.facebook.airlift.log.Level.ERROR;
5352
import static com.facebook.airlift.log.Level.WARN;
53+
import static com.facebook.presto.hive.HiveTestUtils.getDataDirectoryPath;
5454
import static com.facebook.presto.iceberg.CatalogType.HIVE;
5555
import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
5656
import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
@@ -282,38 +282,17 @@ public static void main(String[] args)
282282
throws Exception
283283
{
284284
setupLogging();
285-
Optional<Path> dataDirectory = Optional.empty();
285+
Optional<Path> dataDirectory;
286286
if (args.length > 0) {
287287
if (args.length != 1) {
288288
log.error("usage: IcebergQueryRunner [dataDirectory]\n");
289289
log.error(" [dataDirectory] is a local directory under which you want the iceberg_data directory to be created.]\n");
290290
System.exit(1);
291291
}
292-
293-
File dataDirectoryFile = new File(args[0]);
294-
if (dataDirectoryFile.exists()) {
295-
if (!dataDirectoryFile.isDirectory()) {
296-
log.error("Error: " + dataDirectoryFile.getAbsolutePath() + " is not a directory.");
297-
System.exit(1);
298-
}
299-
else if (!dataDirectoryFile.canRead() || !dataDirectoryFile.canWrite()) {
300-
log.error("Error: " + dataDirectoryFile.getAbsolutePath() + " is not readable/writable.");
301-
System.exit(1);
302-
}
303-
}
304-
else {
305-
// For user supplied path like [path_exists_but_is_not_readable_or_writable]/[paths_do_not_exist], the hadoop file system won't
306-
// be able to create directory for it. e.g. "/aaa/bbb" is not creatable because path "/" is not writable.
307-
while (!dataDirectoryFile.exists()) {
308-
dataDirectoryFile = dataDirectoryFile.getParentFile();
309-
}
310-
if (!dataDirectoryFile.canRead() || !dataDirectoryFile.canWrite()) {
311-
log.error("Error: The ancestor directory " + dataDirectoryFile.getAbsolutePath() + " is not readable/writable.");
312-
System.exit(1);
313-
}
314-
}
315-
316-
dataDirectory = Optional.of(dataDirectoryFile.toPath());
292+
dataDirectory = getDataDirectoryPath(Optional.of(args[0]));
293+
}
294+
else {
295+
dataDirectory = getDataDirectoryPath(Optional.empty());
317296
}
318297

319298
Map<String, String> properties = ImmutableMap.of("http-server.http.port", "8080");

0 commit comments

Comments
 (0)