You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "Tsukilc (via GitHub)" <gi...@apache.org> on 2025/06/10 08:58:43 UTC
[PR] refactor(version): Delete the package existing in java8 [incubator-hugegraph]
Tsukilc opened a new pull request, #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792
<!--
Thank you very much for contributing to Apache HugeGraph, we are happy that you want to help us improve it!
Here are some tips for you:
1. If this is your first time, please read the [contributing guidelines](https://github.com/apache/hugegraph/blob/master/CONTRIBUTING.md)
2. If a PR fix/close an issue, type the message "close xxx" (xxx is the link of related
issue) in the content, GitHub will auto link it (Required)
3. Name the PR title in "Google Commit Format", start with "feat | fix | perf | refactor | doc | chore",
such like: "feat(core): support the PageRank algorithm" or "fix: wrong break in the compute loop" (module is optional)
skip it if you are unsure about which is the best component.
4. One PR address one issue, better not to mix up multiple issues.
5. Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]` (or click it directly after
published)
-->
## Purpose of the PR
sub task of [[#2724](https://github.com/apache/incubator-hugegraph/issues/2724)](https://github.com/apache/incubator-hugegraph/issues/2764)
No longer maintain compatibility with java8 and delete the sun.reflection-related reflection packages of java8
<!--
Please explain more context in this section, clarify why the changes are needed.
e.g:
- If you propose a new API, clarify the use case for a new API.
- If you fix a bug, you can clarify why it is a bug, and should be associated with an issue.
-->
## Main Changes
<!-- Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. These change logs are helpful for better and faster reviews.)
For example:
- If you introduce a new feature, please show detailed design here or add the link of design documentation.
- If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
- If there is a discussion in the mailing list, please add the link. -->
## Verifying these changes
<!-- Please pick the proper options below -->
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [x] Already covered by existing tests, such as *(please modify tests here)*.
- [ ] Need tests and can be verified as follows:
- xxx
## Does this PR potentially affect the following parts?
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] Dependencies ([add/update license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses) info & [regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh)) <!-- Don't forget to add/update the info in "LICENSE" & "NOTICE" files (both in root & dist module) -->
- [ ] Modify configurations
- [ ] The public API
- [x] Other affects (Incompatible with java8)
- [ ] Nope
## Documentation Status
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `Doc - TODO` <!-- Your PR changes impact docs and you will update later -->
- [ ] `Doc - Done` <!-- Related docs have been already added or updated -->
- [x] `Doc - No Need` <!-- Your PR changes don't impact/need docs -->
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] refactor: Delete the package existing in java8 [incubator-hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792#discussion_r2347415618
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java:
##########
@@ -636,8 +633,10 @@ public static void optionalMethodsToFilter(String className, String... methodNam
try {
clazz = Class.forName(className);
} catch (ClassNotFoundException e) {
- // TODO: we just ignore the exception, change it after we drop Java8 support
- LOG.warn("Skip register class {} to filter", className);
+ LOG.debug(
+ "Internal class {} not found in this JDK implementation, skipping filter " +
+ "registration",
+ className, e);
Review Comment:
format
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] refactor: Delete the package existing in java8 [incubator-hugegraph]
Posted by "Pengzna (via GitHub)" <gi...@apache.org>.
Pengzna commented on code in PR #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792#discussion_r2380833885
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/Reflection.java:
##########
@@ -33,49 +33,36 @@ public class Reflection {
private static final Method REGISTER_FILEDS_TO_FILTER_METHOD;
private static final Method REGISTER_METHODS_TO_FILTER_METHOD;
- public static final String JDK_INTERNAL_REFLECT_REFLECTION = "jdk.internal.reflect.Reflection";
- public static final String SUN_REFLECT_REFLECTION = "sun.reflect.Reflection";
Review Comment:
why remove this constant variables?
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] refactor: remove the package existing in java8 [incubator-hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin merged PR #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] refactor: Delete the package existing in java8 [incubator-hugegraph]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792#discussion_r2343059642
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/util/Reflection.java:
##########
@@ -33,49 +33,36 @@ public class Reflection {
private static final Method REGISTER_FILEDS_TO_FILTER_METHOD;
private static final Method REGISTER_METHODS_TO_FILTER_METHOD;
- public static final String JDK_INTERNAL_REFLECT_REFLECTION = "jdk.internal.reflect.Reflection";
- public static final String SUN_REFLECT_REFLECTION = "sun.reflect.Reflection";
-
static {
Method registerFieldsToFilterMethodTemp = null;
Method registerMethodsToFilterMethodTemp = null;
Class<?> reflectionClazzTemp = null;
try {
- reflectionClazzTemp = Class.forName(JDK_INTERNAL_REFLECT_REFLECTION);
+ reflectionClazzTemp = Class.forName("jdk.internal.reflect.Reflection");
+
+ registerFieldsToFilterMethodTemp =
+ reflectionClazzTemp.getMethod("registerFieldsToFilter",
+ Class.class, String[].class);
+
+ registerMethodsToFilterMethodTemp =
+ reflectionClazzTemp.getMethod("registerMethodsToFilter",
+ Class.class, String[].class);
} catch (ClassNotFoundException e) {
- try {
- reflectionClazzTemp = Class.forName(SUN_REFLECT_REFLECTION);
- } catch (ClassNotFoundException ex) {
- LOG.error("Can't find Reflection class", ex);
- }
+ LOG.error("Can't find jdk.internal.reflect.Reflection class, " +
+ "please ensure you are using Java 11", e);
+ } catch (NoSuchMethodException e) {
+ LOG.error("Can't find reflection filter methods", e);
}
Review Comment:
The method lookup logic should be wrapped in a try-catch block to handle NoSuchMethodException separately from ClassNotFoundException, ensuring proper error handling for each failure scenario.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeFactoryAuthProxy.java:
##########
@@ -636,8 +634,8 @@ public static void optionalMethodsToFilter(String className, String... methodNam
try {
clazz = Class.forName(className);
} catch (ClassNotFoundException e) {
- // TODO: we just ignore the exception, change it after we drop Java8 support
- LOG.warn("Skip register class {} to filter", className);
+ LOG.debug("Internal class {} not found in this JDK implementation, skipping filter registration",
+ className, e);
Review Comment:
The log message has a grammatical error. 'Internal class {} not found in this JDK implementation' should be 'Internal class {} not found in this JDK implementation,'.
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] refactor: Delete the package existing in java8 [incubator-hugegraph]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792#issuecomment-3059080755
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] refactor: Delete the package existing in java8 [incubator-hugegraph]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2792:
URL: https://github.com/apache/incubator-hugegraph/pull/2792#issuecomment-3289670324
## [Codecov](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `50.00000%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 34.01%. Comparing base ([`0946d5d`](https://app.codecov.io/gh/apache/incubator-hugegraph/commit/0946d5dd0cad8c27cb365b0c2aaaf217cdca3402?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8ccff76`](https://app.codecov.io/gh/apache/incubator-hugegraph/commit/8ccff76850b577d188cf16488c3ff7f08dbaecc8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ain/java/org/apache/hugegraph/util/Reflection.java](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Futil%2FReflection.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL3V0aWwvUmVmbGVjdGlvbi5qYXZh) | 50.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rg/apache/hugegraph/auth/HugeFactoryAuthProxy.java](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Fauth%2FHugeFactoryAuthProxy.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWdlZ3JhcGgvYXV0aC9IdWdlRmFjdG9yeUF1dGhQcm94eS5qYXZh) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (0946d5d) and HEAD (8ccff76). Click for more details.
>
> <details><summary>HEAD has 2 uploads less than BASE</summary>
>
>| Flag | BASE (0946d5d) | HEAD (8ccff76) |
>|------|------|------|
>||3|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #2792 +/- ##
============================================
- Coverage 40.91% 34.01% -6.90%
- Complexity 333 500 +167
============================================
Files 747 758 +11
Lines 60168 61308 +1140
Branches 7683 7879 +196
============================================
- Hits 24615 20856 -3759
- Misses 32975 38169 +5194
+ Partials 2578 2283 -295
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2792?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org