mosche commented on code in PR #24862:
URL: https://github.com/apache/beam/pull/24862#discussion_r1068089042
##########
runners/spark/3/src/main/java/org/apache/beam/runners/spark/structuredstreaming/translation/SparkSessionFactory.java:
##########
@@ -130,19 +170,35 @@ private static SparkSession.Builder sessionBuilder(
// mode, so try to align with value of "sparkMaster" option in this case.
// We should not overwrite this value (or any user-defined spark
configuration value) if the
// user has already configured it.
- if (master != null
- && !master.equals("local[*]")
- && master.startsWith("local[")
- && System.getProperty("spark.sql.shuffle.partitions") == null) {
- int numPartitions =
- Integer.parseInt(master.substring("local[".length(), master.length()
- 1));
- if (numPartitions > 0) {
- sparkConf.set("spark.sql.shuffle.partitions",
String.valueOf(numPartitions));
- }
+ int partitions = localNumPartitions(master);
+ if (partitions > 0) {
+ sparkConf.setIfMissing("spark.sql.shuffle.partitions",
Integer.toString(partitions));
}
+
return SparkSession.builder().config(sparkConf);
}
+ @SuppressWarnings({"return", "toarray.nullable.elements",
"methodref.receiver"}) // safe to ignore
+ private static String[] filesToStage(
+ SparkStructuredStreamingPipelineOptions opts, Collection<String>
excludes) {
+ Collection<String> files = opts.getFilesToStage();
+ if (files == null || files.isEmpty()) {
+ return EMPTY_STRING_ARRAY;
+ }
+ if (!excludes.isEmpty()) {
+ files = Collections2.filter(files, f ->
!excludes.stream().anyMatch(f::contains));
+ }
+ return files.toArray(EMPTY_STRING_ARRAY);
+ }
+
+ private static String[] sparkJars(SparkConf conf) {
Review Comment:
I'm sorry @aromanenko-dev, you got me started about this 😛
I personally consider the "a method must contain a verb" convention out of
time and even modern Java (fluent builders finally skipping the meaningless
`with` prefix, record types getting rid of the `get` prefix) is slowly getting
there. In my personal view a prefix like `get`, `retrieve`, `do`, `return` has
close to no meaning (plus it doesn't really support understanding) and,
consequently, is better skipped. Luckily, the syntax makes it crystal clear
that this is a method and not a variable nor constant, so we don't have to rely
on placeholder verbs :)
But fair argument below, that method does in deed filter as well - that is a
difference worth mentioning using a verb in my eyes :)
But never mind, i can change to `getSparkJars` - not a big deal 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
