You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/05/10 18:01:42 UTC

[GitHub] [druid] vogievetsky opened a new pull request #11228: Web console: Fix required field treatment

vogievetsky opened a new pull request #11228:
URL: https://github.com/apache/druid/pull/11228


   This PR fixes a bug in the data loader flow when connecting Kinesis.
   
   The issue is that the `Endpoint` field looks like it is filled in with the default value but it also is `required` so it requires input.
   The issue is fixed as a one line change in `ingestion-spec.tsx` per the docs the `endpoint` field is not required.
   
   In addition to fixing this issue:
   
   - AutoForm has been upgraded so that `defaultValue` is ignored when `required` is evaluated to true
   - Tests have been greatly expanded
   - I scanned the code for more instances of this issue and found one in the `TIMESTAMP_SPEC_FIELDS`
   - Added the ability to Alt+Shift+Click a tile in the ingestion screen to go to its connection step even if the extension is not loaded.
   
   This PR has:
   - [x] been self-reviewed.
   - [x] been tested in a test Druid cluster.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] suneet-s commented on pull request #11228: Web console: Fix required field treatment

Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11228:
URL: https://github.com/apache/druid/pull/11228#issuecomment-837425156


   @vogievetsky Do we know when this bug was introduced? I'm wondering if this is bad enough to justify backporting this to 0.21.1 release.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] vogievetsky commented on pull request #11228: Web console: Fix required field treatment

Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #11228:
URL: https://github.com/apache/druid/pull/11228#issuecomment-837582521


   @suneet-s the fact that the endpoint field was incorrectly marked as `required` was there from the start of the data loader but until https://github.com/apache/druid/pull/10533 it just resulted in an incorrect border color that people would not even notice. https://github.com/apache/druid/pull/10533 started using the model definitions as validation also and that is when the original bug went from being (from a user perspective) almost unnoticeable to flow breaking. I think it is worth back-porting 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 to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] suneet-s commented on pull request #11228: Web console: Fix required field treatment

Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11228:
URL: https://github.com/apache/druid/pull/11228#issuecomment-837613502


   @clintropolis fyi I added the milestone 0.21.1 to this bug so we can backport it before making the patch release


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] clintropolis merged pull request #11228: Web console: Fix required field treatment

Posted by GitBox <gi...@apache.org>.
clintropolis merged pull request #11228:
URL: https://github.com/apache/druid/pull/11228


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org