-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-24768][FollowUp][SQL]Avro migration followup: change artifactId to spark-avro #21866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| options: Map[String, String], | ||
| files: Seq[FileStatus]): Option[StructType] = { | ||
| val conf = spark.sparkContext.hadoopConfiguration | ||
| val conf = spark.sessionState.newHadoopConf() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add a linter rule for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gengliangwang Please help this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, will do it.
Can I make it a simple one, check the appearance of
spark.sparkContext.hadoopConfiguration ?
Otherwise there are too many usage for the sparkContext.hadoopConfiguration
|
LGTM |
|
Test build #93518 has finished for PR 21866 at commit
|
|
retest this please |
|
Test build #93528 has finished for PR 21866 at commit
|
|
retest this please |
1 similar comment
|
retest this please |
|
ok to test |
|
Test build #93536 has finished for PR 21866 at commit
|
|
retest this please |
|
Test build #93541 has finished for PR 21866 at commit
|
|
Thanks! Merged to master. |
…Id to spark-avro After rethinking on the artifactId, I think it should be `spark-avro` instead of `spark-sql-avro`, which is simpler, and consistent with the previous artifactId. I think we need to change it before Spark 2.4 release. Also a tiny change: use `spark.sessionState.newHadoopConf()` to get the hadoop configuration, thus the related hadoop configurations in SQLConf will come into effect. Unit test Author: Gengliang Wang <gengliang.wang@databricks.com> Closes apache#21866 from gengliangwang/avro_followup. (cherry picked from commit c44eb56)
What changes were proposed in this pull request?
After rethinking on the artifactId, I think it should be
spark-avroinstead ofspark-sql-avro, which is simpler, and consistent with the previous artifactId. I think we need to change it before Spark 2.4 release.Also a tiny change: use
spark.sessionState.newHadoopConf()to get the hadoop configuration, thus the related hadoop configurations in SQLConf will come into effect.How was this patch tested?
Unit test