Nessie: Extract Spark AppId / User from SparkContext and not from Snapshot#2664
Nessie: Extract Spark AppId / User from SparkContext and not from Snapshot#2664rymurr merged 3 commits intoapache:masterfrom
Conversation
nastra
force-pushed
the
extract-spark-appid-user
branch
from
f69d25b to
656185c
Compare
nastra
changed the title
nastra
force-pushed
the
extract-spark-appid-user
branch
from
656185c to
80f5692
Compare
nastra
force-pushed
the
extract-spark-appid-user
branch
from
80f5692 to
95fc018
Compare
Also if you have any thoughts on allowing passthrough of options to catalogs @rymurr, please feel free to comment on this issue: #2607 The issue is specifically about hive options but seems like in general the Nessie catalog might have related concerns based on the above quote. cc @nastra as well if you have input. |
|
@kbendick thanks for mentioning #2607. I've read through it and I don't have any better suggestions than what was already mentioned on #2607. Re the similary of #2607 and #2664: both are kind of related, but different at the same time. #2607 is more about overriding hadoop config stuff, whereas #2664 is about extracting some info at some point and passing it down via the existing properties. |
|
@rdblue given that this approach came up in https://github.com/apache/iceberg/pull/1587/files#r520825564, would you also like to weigh in here? |
nessie/src/main/java/org/apache/iceberg/nessie/NessieTableOperations.java
Outdated
Show resolved
Hide resolved
| return new NessieTableOperations(NessieUtil.toKey(pti.tableIdentifier()), newReference, client, fileIO); | ||
| NessieTableOperations tableOperations = | ||
| new NessieTableOperations(NessieUtil.toKey(pti.tableIdentifier()), newReference, client, fileIO); | ||
| // TODO: is there a better way to pass the catalog options to the TableOperations than this? |
There was a problem hiding this comment.
Add it to the constructor? The constructor is public, but I don't think there is a need for it to be public. You could create a package-private one that is called here instead. (Maybe @rymurr can also comment)
There was a problem hiding this comment.
for some reason I assumed that the signature of the constructor was defined and shouldn't be changed. Definitely better to pass this via the constructor
There was a problem hiding this comment.
@rymurr, is there a reason why the constructor was public?
There was a problem hiding this comment.
I think it was public because it was created before CatalogUtil.loadCatalog. No real reason for it to be public now.
There was a problem hiding this comment.
ahhh excuse me, you meant the TableOps. That was just a mistake I suspect. Even less of a reason for that to be public.
spark3/src/main/java/org/apache/iceberg/spark/SparkCatalog.java
Outdated
Show resolved
Hide resolved
…ot from Snapshot As mentioned in https://github.com/apache/iceberg/pull/1587/files#r520825564 it is probably better to extract the necessary Spark info directly from the `SparkContext` and pass it down via the `CaseInsensitiveStringMap`
nastra
force-pushed
the
extract-spark-appid-user
branch
from
95fc018 to
f6bf1ac
Compare
…t and not from Snapshot
nastra
force-pushed
the
extract-spark-appid-user
branch
from
f6bf1ac to
94cc6dd
Compare
spark3/src/main/java/org/apache/iceberg/spark/SparkCatalog.java
Outdated
Show resolved
Hide resolved
…t and not from Snapshot
4 participants
No description provided.