You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/03/09 07:02:06 UTC
[GitHub] [druid] himanshug opened a new pull request #10964: Free Druid source code from Guava by shading
himanshug opened a new pull request #10964:
URL: https://github.com/apache/druid/pull/10964
Fixes #6942
### Description
This might look like a large PR but most changes are mechanical i.e. updates to import statements and guava dependency in various pom files.
Here is the meaty part...
1. A new module "shaded-guava" is introduced that produces classes in "org.apache.druid.com.google.common" package by shading real guava. See `shaded-guava/pom.xml` .
2. pom.xml in all modules are made to depend upon shaded guava jar above instead of real guava, that change looks like
```
- <groupId>com.google.guava</groupId>
+ <groupId>org.apache.druid.com.google.guava</groupId>
```
3. In all Java files, `import com.google.common.X` is replaced by `import org.apache.druid.com.google.guava.X`
4. "Organize Imports" is done to all Java files to correct import ordering or checkstyles fails
5. Some imports in `cloudfiles-extensions` and `sql` modules are reverted to use `import com.google.common.X` because their dependencies `jclouds-core` and `calcite` respectively mandate using real `import com.google.common.X` in various methods exposed by those libraries. But this is minimal.
6. some imports that used `org.apache.curator.shaded.com.google.common.X` , i.e. shaded version of guava used by curator, continue to use same. that is independent.
(this approach was briefly described in https://github.com/apache/druid/issues/6942#issuecomment-791029487 )
End Result is that Druid source code is free from real guava and can use any version of guava inside `shaded-guava` module, few exceptions in (5) as described above but that is minimal and calcite e.g. ensures that its usage of guava is minimal so as to be able to support most guava versions.
we still ship guava-0.16.1.jar like before, so no external integrations should see any impact. User should be able to switch this with their favorite version of guava jar at runtime and Druid source code be fine because it uses druid-shaded-guava-0.x.y.jar
Existing Unit, Integration Tests and Travis builds should cover all of the changes except for Hadoop Integration that I could not verify due to not having quick access to such setup. However, given they way this patch works, Hadoop Integration should see no change.
<hr>
<!-- Check the items by putting "x" in the brackets for the done things. Not all of these items apply to every PR. Remove the items which are not done or not relevant to the PR. None of the items from the checklist below are strictly necessary, but it would be very helpful if you at least self-review the PR. -->
This PR has:
- [x] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-845512253
> Looks good, .. I wonder if extensions-core/hive-extensions/pom.xml was added by mistake?
yes
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-845512253
> Looks good, .. I wonder if extensions-core/hive-extensions/pom.xml was added by mistake?
yes, thanks for pointing out
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-1076924170
@egor-ryashin Thanks. Unfortunately I will not be able to spend much time on it in the near future. Please feel free to take the commits here , create a new PR with conflicts resolved and any other files that were added later.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] mghosh4 commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
mghosh4 commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-804297541
This looks good to me. @himanshug when are we planning to commit this?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] lgtm-com[bot] commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
lgtm-com[bot] commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-793535499
This pull request **fixes 7 alerts** when merging 9f55cbf1297d8efbd0a823c079cf3d0ccc2a5c03 into 0f81ce32a082b78d32cb60a9cc57879d16340978 - [view on LGTM.com](https://lgtm.com/projects/g/apache/druid/rev/pr-f2cfc1dc91a6a493ed9f3e59f7709758358462f5)
**fixed alerts:**
* 7 for Array index out of bounds
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-793468970
Ah, this PR is very prone to merge conflicts due to the sweeping changes made. However, this PR can be reviewed in the presence of conflicts [with mechanical changes described in PR description]. I will fix all the conflicts when this PR is accepted and ready to be merged.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
himanshug commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-806232950
@mghosh4 needs to get review/approval before it can be merged.
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] egor-ryashin edited a comment on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
egor-ryashin edited a comment on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-1073313908
We need to merge the ticket, it will solve a bunch of problems and people waiting for it. Please, ping me, if you need help with the merge.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] egor-ryashin commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
egor-ryashin commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-845211849
Looks good, @himadrisingh I wonder if `extensions-core/hive-extensions/pom.xml` was added by mistake?
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] egor-ryashin commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
egor-ryashin commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-1073313908
We need to merge the ticket, it will solve a bunch of problems and people waiting for it.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] egor-ryashin removed a comment on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
egor-ryashin removed a comment on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-845231318
Looks like `ambari-metrics-emitter` will fail without guava 16:
```
mvn dependency:tree
...
[INFO] +- org.apache.ambari:ambari-metrics-common:jar:2.7.0.0.0:compile
[INFO] | +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] | +- com.google.code.gson:gson:jar:2.2.2:compile
[INFO] | +- com.google.guava:guava:jar:16.0.1:compile
```
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] egor-ryashin commented on pull request #10964: Free Druid source code from Guava by shading
Posted by GitBox <gi...@apache.org>.
egor-ryashin commented on pull request #10964:
URL: https://github.com/apache/druid/pull/10964#issuecomment-845231318
Looks like `ambari-metrics-emitter` will fail without guava 16:
```
mvn dependency:tree
...
[INFO] +- org.apache.ambari:ambari-metrics-common:jar:2.7.0.0.0:compile
[INFO] | +- commons-logging:commons-logging:jar:1.1.1:compile
[INFO] | +- com.google.code.gson:gson:jar:2.2.2:compile
[INFO] | +- com.google.guava:guava:jar:16.0.1:compile
```
--
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org