aokolnychyi commented on a change in pull request #2287:
URL: https://github.com/apache/iceberg/pull/2287#discussion_r589880210
##########
File path: spark/src/main/java/org/apache/iceberg/actions/BaseSparkAction.java
##########
@@ -50,6 +53,24 @@
protected abstract Table table();
+ protected abstract R doExecute();
+
+ protected abstract JobGroupInfo jobGroup();
Review comment:
As much as I like having this in a single place, I am not sure this will
work for actions that trigger multiple jobs. For example, data compaction
@RussellSpitzer is working on will submit a job per partition and we would want
to a separate description each time.
I've just merged a refactoring of `BaseSparkAction` that now makes
SparkContext accessible in this class.
I think we can have this:
```
protected <T> T withJobGroupInfo(JobGroupInfo info, Supplier<R> supplier) {
JobGroupInfo previousInfo = JobGroupUtils.getJobGroupInfo(sparkContext);
try {
JobGroupUtils.setJobGroupInfo(sparkContext, info);
return supplier.get();
} finally {
JobGroupUtils.setJobGroupInfo(sparkContext, previousInfo);
}
}
```
Then, yes, we would need to modify more code in the existing actions but it
should not be that bad.
```
public ... execute() {
JobGroupInfo info = ...
withJobGroupInfo(info, () -> {
// call doExecute (which is now points to the code in execute)
})
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
