You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by sohami <gi...@git.apache.org> on 2018/01/15 07:30:45 UTC
[GitHub] drill pull request #1092: DRILL-6088: MainLoginPageModel errors out when htt...
GitHub user sohami opened a pull request:
https://github.com/apache/drill/pull/1092
DRILL-6088: MainLoginPageModel errors out when http.auth.mechanisms i…
…s not configured
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sohami/drill DRILL-6088
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1092.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1092
----
commit 93bd4d5b3516107157f91b22530639ad7b8ecf8b
Author: Sorabh Hamirwasia <sh...@...>
Date: 2018-01-15T06:25:51Z
DRILL-6088: MainLoginPageModel errors out when http.auth.mechanisms is not configured
----
---
[GitHub] drill issue #1092: DRILL-6088: MainLoginPageModel errors out when http.auth....
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the issue:
https://github.com/apache/drill/pull/1092
+1, LGTM.
---
[GitHub] drill issue #1092: DRILL-6088: MainLoginPageModel errors out when http.auth....
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on the issue:
https://github.com/apache/drill/pull/1092
@arina-ielchiieva - I have rebased the commit's on latest master and made the change as suggested. I have also made the MainLoginPageModel class public since it was causing the issue without that at runtime. Looks like during testing I kept it as public but later while opening the PR I might have changed it to package-private and was seeing issues today again during test.
---
[GitHub] drill pull request #1092: DRILL-6088: MainLoginPageModel errors out when htt...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/1092#discussion_r162331112
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/LogInLogOutResources.java ---
@@ -132,24 +133,26 @@ public Viewable getMainLoginPage(@Context HttpServletRequest request, @Context H
@Context SecurityContext sc, @Context UriInfo uriInfo,
@QueryParam(WebServerConstants.REDIRECT_QUERY_PARM) String redirect) throws Exception {
updateSessionRedirectInfo(redirect, request);
- final DrillConfig drillConfig = workManager.getContext().getConfig();
- MainLoginPageModel model = new MainLoginPageModel(null, drillConfig);
+ final MainLoginPageModel model = new MainLoginPageModel(null);
return ViewableWithPermissions.createMainLoginPage(model);
}
- private class MainLoginPageModel {
+ @VisibleForTesting
+ class MainLoginPageModel {
private final String error;
private final boolean authEnabled;
+ private final DrillConfig config;
--- End diff --
It looks like you are using config only in constructor, so it can not store it in class.
---
[GitHub] drill pull request #1092: DRILL-6088: MainLoginPageModel errors out when htt...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1092
---
[GitHub] drill issue #1092: DRILL-6088: MainLoginPageModel errors out when http.auth....
Posted by sohami <gi...@git.apache.org>.
Github user sohami commented on the issue:
https://github.com/apache/drill/pull/1092
@arina-ielchiieva - Can you please help to review this PR ?
---