szetszwo merged PR #6739:
URL: https://github.com/apache/hadoop/pull/6739
--
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:
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2110845650
@steveloughran , thanks a lot for reviewing 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
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2110842717
> continuous-integration/jenkins/pr-head Pending — This commit is being built
This has been stuck by `Windows Batch Script` for more than 22 hours. Since
this passed all the
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2108804028
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
szetszwo commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1598897001
##
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoUtils.java:
##
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2102051696
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
_ Prechecks
steveloughran commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1584770438
##
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoUtils.java:
##
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2083295687
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
steveloughran commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1581033141
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoUtils.java:
##
@@ -55,15 +58,18 @@ public static String
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2078071797
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2077652244
Oops, just found that I accidentally changed the whitespaces in
`TestCryptoCodec`. Let me revert them.
--
This is an automated message from the Apache Git Service.
To respond to the
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2076112558
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
szetszwo commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1578474234
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoUtils.java:
##
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation
szetszwo commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1578470256
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoUtils.java:
##
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2073325889
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
steveloughran commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1576632095
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoUtils.java:
##
@@ -19,53 +19,63 @@
import
szetszwo commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1576476989
##
hadoop-common-project/hadoop-common/src/main/resources/core-default.xml:
##
@@ -3625,7 +3625,19 @@ The switch to turn S3A auditing on or off.
The JCE provider
szetszwo commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1576476164
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoUtils.java:
##
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2072565481
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
_ Prechecks
steveloughran commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1576126175
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoUtils.java:
##
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2069920257
Let's do Alternative Approach 2 then. We could as welll add another conf to
disable the existing hardcoded provider.
--
This is an automated message from the Apache Git Service.
To
steveloughran commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2069888269
yeah, looks incompatible. I'd rather it supports switching, but the existing
BC jar ships and everything works
--
This is an automated message from the Apache Git Service.
To
ayushtkn commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2067367591
Passing by...
To me Alt-1 looks functionally incompatible.
As of today If my setup is working out of the box, I upgrade Hadoop, It will
start giving me ``ClassNotFound``
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2067272199
Let me clarify in more details:
- Current code without any changes:
- Force everyone to add the `bcprov-jdk18on` security provider
- Current PR (remove `bcprov-jdk18on`
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2067264884
> will this require changes everywhere? or will the default of bce still
work? ...
If we change it to use java reflection (similar to
`fs.AbstractFileSystem.hdfs.impl`), then it
steveloughran commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2067046232
will this require changes everywhere? or will the default of bce still work?
as if it gives a choice, fine -just as long as it doesn't force that choice on
everyone
--
This is an
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2061850531
@steveloughran , question to you:
```java
+++ b/hadoop-common-project/hadoop-common/pom.xml
@@ -375,6 +375,7 @@
org.bouncycastle
bcprov-jdk18on
+
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2061830741
> ... Actually, I think it was minihdfs which needed it for tests, rather
than the actual code
Yes, there are many tests requiring `BouncyCastle`. They were not changed
in the
steveloughran commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2061803874
> hadoop-azure seems okay? The tests did not fail.
we'd have to see about the integration tests. Actually, I think it was
minihdfs which needed it for tests, rather than the
szetszwo commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1569133307
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProvider.java:
##
@@ -35,7 +34,6 @@
import java.util.Map;
import
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2060278478
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2060158389
> there are downstream modules which depend on bc from hadoop common. they
need to be adjusted so their poms declare explicit use. ...
It make sense for the downstream modules
szetszwo commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2060155541
> the pr cuts out all bouncy castle init. What will break?
@steveloughran , thanks for looking at this! As mentioned in
`core-default.xml`, could we ask admin to set
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2060001044
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
hadoop-yetus commented on PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#issuecomment-2059994340
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: |
steveloughran commented on code in PR #6739:
URL: https://github.com/apache/hadoop/pull/6739#discussion_r1567940219
##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProvider.java:
##
@@ -35,7 +34,6 @@
import java.util.Map;
import
36 matches
Mail list logo
