lgtm

I am concerned that this will introduce extra dependencies that may conflict with Spark runtime jars. Could you analysis which jars are introduced and could we shade them?

I am concerned that this will introduce extra dependencies that may conflict with Spark runtime jars. Could you analysis which jars are introduced and could we shade them?

@FANNG1 The dependency of caffeine is relatively simple, and there are currently no packages that conflict with Spark. Spark itself does not depend on caffeine, so there should be no need shade them yet

@jerryshao WDYT?

I am concerned that this will introduce extra dependencies that may conflict with Spark runtime jars. Could you analysis which jars are introduced and could we shade them?

@FANNG1 The dependency of caffeine is relatively simple, and there are currently no packages that conflict with Spark. Spark itself does not depend on caffeine, so there should be no need shade them yet

You should shade caffeine Because caffeine may be used by other projects like Iceberg. Caffeine is a common libary.

I am concerned that this will introduce extra dependencies that may conflict with Spark runtime jars. Could you analysis which jars are introduced and could we shade them?

@FANNG1 The dependency of caffeine is relatively simple, and there are currently no packages that conflict with Spark. Spark itself does not depend on caffeine, so there should be no need shade them yet

You should shade caffeine Because caffeine may be used by other projects like Iceberg. Caffeine is a common libary.

ok, I got it, have another question, Do all references in Gravitino need to be shaded ?

I am concerned that this will introduce extra dependencies that may conflict with Spark runtime jars. Could you analysis which jars are introduced and could we shade them?

@FANNG1 The dependency of caffeine is relatively simple, and there are currently no packages that conflict with Spark. Spark itself does not depend on caffeine, so there should be no need shade them yet

You should shade caffeine Because caffeine may be used by other projects like Iceberg. Caffeine is a common libary.

ok, I got it, have another question, Do all references in Gravitino need to be shaded ?

You just need to shade the common libraries.

@jerqi I understand that it should be adding a 'spark-common-runtime' module, right?

@jerqi I understand that it should be adding a 'spark-common-runtime' module, right?

No need. We should include spark-common jar in spark-runtime jar.

From a long term perspective, we are really supposed to be cautious when introducing new dependencies. We may need to revisit our existing ones and see if those are necessary.
Most modern software developed in Java, Python, Go, Rust etc have a messy graph of package dependencies. This is unavoidable, but still something we as a team should watch out.
It is perfectly fine to introduce an external dependency when the benefits we gain from it is justified. But bear in mind, all of them are animals in our zoo, and we are gonna take care of it since the day it is incorporated.
I'm too old-fashioned to appreciate this kind of wide-range dependencies, for fear of maintenance burden, supply chain attacks, upgrade efforts, to name a few.

From a long term perspective, we are really supposed to be cautious when introducing new dependencies. We may need to revisit our existing ones and see if those are necessary. Most modern software developed in Java, Python, Go, Rust etc have a messy graph of package dependencies. This is unavoidable, but still something we as a team should watch out. It is perfectly fine to introduce an external dependency when the benefits we gain from it is justified. But bear in mind, all of them are animals in our zoo, and we are gonna take care of it since the day it is incorporated. I'm too old-fashioned to appreciate this kind of wide-range dependencies, for fear of maintenance burden, supply chain attacks, upgrade efforts, to name a few.

I'm inclined to the point of @tengqm , I doubt whether it's worthwhile to unify the cache framework by introducing another dependency.

@tengqm @FANNG1 Usually, only one caching method is kept in a project, which makes it convenient for us to review or solve only one type of problem when it occurs, including reference issues, vulnerability issues, or other usage issues. Of course, this is not a mandatory option.

@tengqm @FANNG1 Usually, only one caching method is kept in a project, which makes it convenient for us to review or solve only one type of problem when it occurs, including reference issues, vulnerability issues, or other usage issues. Of course, this is not a mandatory option.

Cool. Then this PR is actually unifying caching to a single package?
A big yes from me then.