Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16833 )

Change subject: IMPALA-10496: SAML implementation in Impala
......................................................................


Patch Set 22:

(37 comments)

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/AuthTokenGenerator.java
File 
fe/src/main/java/org/apache/impala/authentication/saml/AuthTokenGenerator.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/AuthTokenGenerator.java@20
PS22, Line 20: // copy of 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/AuthTokenGenerator.java
line too long (135 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java
File 
fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java@32
PS22, Line 32: // 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlAuthTokenGenerator.java
line too long (135 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlAuthTokenGenerator.java@52
PS22, Line 52:   private static final Logger LOG = 
LoggerFactory.getLogger(HiveSamlAuthTokenGenerator.class);
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlGroupNameFilter.java
File 
fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlGroupNameFilter.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlGroupNameFilter.java@31
PS22, Line 31: // 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlGroupNameFilter.java
line too long (132 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateInfo.java
File 
fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateInfo.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateInfo.java@20
PS22, Line 20: // copy of 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlRelayStateInfo.java
line too long (139 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java
File 
fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/HiveSamlRelayStateStore.java@32
PS22, Line 32: // slightly modified copy of 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSamlRelayStateInfo.java
line too long (157 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java
File 
fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@43
PS22, Line 43: // modified version of 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/auth/saml/HiveSaml2Client.java
line too long (144 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@69
PS22, Line 69:     //TODO handle the replayCache as described in 
http://www.pac4j.org/docs/clients/saml.html
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@148
PS22, Line 148:     // This is done to keep original structure by Vihang + keep 
ImpalaSamlClient as the only
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@188
PS22, Line 188:   // 
https://github.com/vihangk1/hive/blob/master_saml/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
line too long (129 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/authentication/saml/ImpalaSamlClient.java@189
PS22, Line 189:   private String doSamlAuth(WrappedWebContext webContext) 
throws HttpSamlAuthenticationException {
line too long (98 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/service/BackendConfig.java
File fe/src/main/java/org/apache/impala/service/BackendConfig.java:

http://gerrit.cloudera.org:8080/#/c/16833/22/fe/src/main/java/org/apache/impala/service/BackendConfig.java@237
PS22, Line 237:   public String getSaml2GroupAttibuteName() { return 
backendCfg_.saml2_group_attribute_name; }
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py
File tests/custom_cluster/test_saml2_sso.py:

http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@21
PS22, Line 21: import logging
flake8: F401 'logging' imported but unused


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@31
PS22, Line 31: from tests.common.impala_service import ImpaladService
flake8: F401 'tests.common.impala_service.ImpaladService' imported but unused


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@34
PS22, Line 34: from tests.shell.util import run_impala_shell_cmd, 
run_impala_shell_cmd_no_expect, \
flake8: F401 'tests.shell.util.run_impala_shell_cmd_no_expect' imported but 
unused


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@34
PS22, Line 34: from tests.shell.util import run_impala_shell_cmd, 
run_impala_shell_cmd_no_expect, \
flake8: F401 'tests.shell.util.ImpalaShell' imported but unused


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@37
PS22, Line 37: class NoRedirection(urllib2.HTTPErrorProcessor):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@47
PS22, Line 47: class TestClientSaml(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@71
PS22, Line 71: \
flake8: E502 the backslash is redundant between brackets


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@188
PS22, Line 188:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@188
PS22, Line 188:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@189
PS22, Line 189:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@189
PS22, Line 189:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@190
PS22, Line 190:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@190
PS22, Line 190:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@191
PS22, Line 191:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@191
PS22, Line 191:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@192
PS22, Line 192:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@192
PS22, Line 192:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@193
PS22, Line 193:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@193
PS22, Line 193:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@194
PS22, Line 194:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@194
PS22, Line 194:
flake8: E251 unexpected spaces around keyword / parameter equals


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@274
PS22, Line 274: #
flake8: E116 unexpected indentation (comment)


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@277
PS22, Line 277: a
flake8: E501 line too long (96 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@280
PS22, Line 280: a
flake8: E501 line too long (113 > 90 characters)


http://gerrit.cloudera.org:8080/#/c/16833/22/tests/custom_cluster/test_saml2_sso.py@283
PS22, Line 283: :
flake8: E501 line too long (98 > 90 characters)



--
To view, visit http://gerrit.cloudera.org:8080/16833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia0c026cba1b90e7ff6ec5ae49be78b0d1edd8dfa
Gerrit-Change-Number: 16833
Gerrit-PatchSet: 22
Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Feb 2021 15:15:01 +0000
Gerrit-HasComments: Yes

Reply via email to