-
Notifications
You must be signed in to change notification settings - Fork 980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DRILL-6088: MainLoginPageModel errors out when http.auth.mechanisms i… #1092
Conversation
@arina-ielchiieva - Can you please help to review this PR ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good except of one small comment, +1.
|
||
private final String error; | ||
|
||
private final boolean authEnabled; | ||
|
||
private final DrillConfig config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you are using config only in constructor, so it may not store it in class.
@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. |
+1, LGTM. |
…s not configured DRILL-6088: Update based on review feedback close apache#1092
…s not configured