You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/12/20 19:36:25 UTC
[GitHub] [hive] cnauroth opened a new pull request, #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
cnauroth opened a new pull request, #3886:
URL: https://github.com/apache/hive/pull/3886
### What changes were proposed in this pull request?
branch-3 uses Apache Directory Server in some tests. It currently uses version 1.5.6. This version has a transitive dependency to a SNAPSHOT, making it awkward to build and release. We can upgrade to 1.5.7 to remove the SNAPSHOT dependency.
### Why are the changes needed?
SNAPSHOT dependencies make local builds awkward and release build policies typically don't allow them.
### Does this PR introduce _any_ user-facing change?
No, Apache Directory Server is only used for tests.
### How was this patch tested?
I built locally and confirmed tests.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] cnauroth commented on pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
cnauroth commented on PR #3886:
URL: https://github.com/apache/hive/pull/3886#issuecomment-1366271795
> Apart from the version bump the respective commit in master had also some other changes. Is there a reason for leaving them out?
@zabetak , sorry, missed that. I've just pushed up the additional changes. The service/pom.xml change was only partially applicable, because there is no exclusion that needs to be removed on branch-3.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak closed pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
URL: https://github.com/apache/hive/pull/3886
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a diff in pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3886:
URL: https://github.com/apache/hive/pull/3886#discussion_r1059967623
##########
service/pom.xml:
##########
@@ -291,13 +291,6 @@
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.apache.directory.client.ldap</groupId>
- <artifactId>ldap-client-api</artifactId>
- <version>${apache-directory-clientapi.version}</version>
- <scope>test</scope>
- </dependency>
-
<dependency>
<groupId>org.apache.directory.server</groupId>
<artifactId>apacheds-server-integ</artifactId>
Review Comment:
Can you clarify why we don't want to `exclude ldap-client-api` from here as it was done in the original commit in master (https://github.com/apache/hive/commit/5581eb8a74a4f33b35b7bf70d9ec4e9a95f3b8a0).
When I run `mvn dependency:tree` in service module I see the `org.apache.directory.client.ldap:ldap-client-api` dependency coming transitively.
```
[INFO] +- org.apache.directory.server:apacheds-server-integ:jar:1.5.7:test
[INFO] | +- org.apache.directory.server:apacheds-interceptor-kerberos:jar:1.5.7:test
[INFO] | | +- org.apache.directory.server:apacheds-core:jar:1.5.7:test
[INFO] | | | +- org.apache.directory.server:apacheds-core-api:jar:1.5.7:test
[INFO] | | | | +- org.apache.directory.server:apacheds-core-entry:jar:1.5.7:test
[INFO] | | | | \- org.apache.directory.server:apacheds-core-constants:jar:1.5.7:test
[INFO] | | | +- org.apache.directory.server:apacheds-utils:jar:1.5.7:test
[INFO] | | | \- bouncycastle:bcprov-jdk15:jar:140:test
[INFO] | | \- org.apache.directory.server:apacheds-kerberos-shared:jar:1.5.7:test
[INFO] | | +- org.apache.directory.server:apacheds-core-jndi:jar:1.5.7:test
[INFO] | | \- org.apache.directory.server:apacheds-protocol-shared:jar:1.5.7:test
[INFO] | +- org.apache.directory.server:apacheds-core-integ:jar:1.5.7:test
[INFO] | +- ldapsdk:ldapsdk:jar:4.1:test
[INFO] | +- org.apache.directory.client.ldap:ldap-client-api:jar:0.1:test
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a diff in pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3886:
URL: https://github.com/apache/hive/pull/3886#discussion_r1061421382
##########
service/pom.xml:
##########
@@ -291,13 +291,6 @@
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.apache.directory.client.ldap</groupId>
- <artifactId>ldap-client-api</artifactId>
- <version>${apache-directory-clientapi.version}</version>
- <scope>test</scope>
- </dependency>
-
<dependency>
<groupId>org.apache.directory.server</groupId>
<artifactId>apacheds-server-integ</artifactId>
Review Comment:
Oh you are right. I was confused while checking the diff and though that the exclusion was added rather than removed. Apologies for the back and forth!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] zabetak commented on a diff in pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
zabetak commented on code in PR #3886:
URL: https://github.com/apache/hive/pull/3886#discussion_r1059967623
##########
service/pom.xml:
##########
@@ -291,13 +291,6 @@
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.apache.directory.client.ldap</groupId>
- <artifactId>ldap-client-api</artifactId>
- <version>${apache-directory-clientapi.version}</version>
- <scope>test</scope>
- </dependency>
-
<dependency>
<groupId>org.apache.directory.server</groupId>
<artifactId>apacheds-server-integ</artifactId>
Review Comment:
Can you clarify why we don't want to exclude `ldap-client-api` from here as it was done in the original commit in master (https://github.com/apache/hive/commit/5581eb8a74a4f33b35b7bf70d9ec4e9a95f3b8a0).
When I run `mvn dependency:tree` in service module I see the `org.apache.directory.client.ldap:ldap-client-api` dependency coming transitively.
```
[INFO] +- org.apache.directory.server:apacheds-server-integ:jar:1.5.7:test
[INFO] | +- org.apache.directory.server:apacheds-interceptor-kerberos:jar:1.5.7:test
[INFO] | | +- org.apache.directory.server:apacheds-core:jar:1.5.7:test
[INFO] | | | +- org.apache.directory.server:apacheds-core-api:jar:1.5.7:test
[INFO] | | | | +- org.apache.directory.server:apacheds-core-entry:jar:1.5.7:test
[INFO] | | | | \- org.apache.directory.server:apacheds-core-constants:jar:1.5.7:test
[INFO] | | | +- org.apache.directory.server:apacheds-utils:jar:1.5.7:test
[INFO] | | | \- bouncycastle:bcprov-jdk15:jar:140:test
[INFO] | | \- org.apache.directory.server:apacheds-kerberos-shared:jar:1.5.7:test
[INFO] | | +- org.apache.directory.server:apacheds-core-jndi:jar:1.5.7:test
[INFO] | | \- org.apache.directory.server:apacheds-protocol-shared:jar:1.5.7:test
[INFO] | +- org.apache.directory.server:apacheds-core-integ:jar:1.5.7:test
[INFO] | +- ldapsdk:ldapsdk:jar:4.1:test
[INFO] | +- org.apache.directory.client.ldap:ldap-client-api:jar:0.1:test
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] cnauroth commented on pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
cnauroth commented on PR #3886:
URL: https://github.com/apache/hive/pull/3886#issuecomment-1360062672
This was already done on the master branch in [HIVE-21836](https://issues.apache.org/jira/browse/HIVE-21836), authored by @zabetak and approved by @abstractdog . Would one of you be able to review?
CC: @amanraj2520 , FYI on a branch-3 change.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] cnauroth commented on a diff in pull request #3886: HIVE-26880: Upgrade Apache Directory Server to 1.5.7 for release 3.2.
Posted by GitBox <gi...@apache.org>.
cnauroth commented on code in PR #3886:
URL: https://github.com/apache/hive/pull/3886#discussion_r1060901243
##########
service/pom.xml:
##########
@@ -291,13 +291,6 @@
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.apache.directory.client.ldap</groupId>
- <artifactId>ldap-client-api</artifactId>
- <version>${apache-directory-clientapi.version}</version>
- <scope>test</scope>
- </dependency>
-
<dependency>
<groupId>org.apache.directory.server</groupId>
<artifactId>apacheds-server-integ</artifactId>
Review Comment:
Commit 5581eb8a74a4f33b35b7bf70d9ec4e9a95f3b8a0 actually removed the exclusion from the existing codebase. This exclusion is not present in branch-3, so there isn't anything for this patch to remove.
If I run `mvn dependency:tree` on master, I see the same transitive dependency. Looking back on the history of [HIVE-21836](https://issues.apache.org/jira/browse/HIVE-21836), I see there was a Jenkins run for HIVE-21836.01.patch, which did not remove the exclusion, and it had a test failure in `TestLdapAtnProviderWithMiniDS`. Then, HIVE-21836.02.patch removed the exclusion, and it got a successful Jenkins run. I can't reproduce that problem though. I tried adding the exclusion back, and the test still passed fine for me.
Let me know if you think we should bring back this exclusion on both master and branch-3. Thanks!
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org