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/01/24 20:35:23 UTC

[GitHub] [druid] vogievetsky opened a new pull request #10794: Web console: Remove first / last suggestions

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


   Comment out some optimistic suggestion until such a time when they will be possible at ingestion time.
   
   Fixes https://github.com/apache/druid/issues/10702
   
   ![image](https://user-images.githubusercontent.com/177816/105642753-7e035e80-5e40-11eb-802f-1f06ead7a563.png)
   


----------------------------------------------------------------
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 #10794: Web console: Remove first / last suggestions

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


   > @suneet-s I'm working on #10702. I have finished main coding and unit test cases, and is going to add new IT test cases. But it will take me some time to finish it since I don't get a large chunk of time these days. Once the fix is completed, I will bring back the UI removed by this PR.
   
   @FrankChen021 that's great news! Looking forward to the PR!


----------------------------------------------------------------
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 #10794: Web console: Remove first / last suggestions

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


   I don't think this fixes #10702 IMO it should still be an open issue, this fix makes it less likely that someone will trip on this from the UI


----------------------------------------------------------------
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] jihoonson commented on a change in pull request #10794: Web console: Remove first / last suggestions

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #10794:
URL: https://github.com/apache/druid/pull/10794#discussion_r563920310



##########
File path: web-console/src/druid-models/metric-spec.tsx
##########
@@ -67,14 +67,17 @@ export const METRIC_SPEC_FIELDS: Field<MetricSpec>[] = [
         group: 'max',
         suggestions: ['longMax', 'doubleMax', 'floatMax'],
       },
-      {
-        group: 'first',
-        suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
-      },
-      {
-        group: 'last',
-        suggestions: ['longLast', 'doubleLast', 'floatLast'],
-      },
+      // Do not show first and last aggregators as they can not be used in ingestion specs and this definition is only used in the data loader.
+      // Ref: https://druid.apache.org/docs/latest/querying/aggregations.html#first--last-aggregator
+      // Should the first / last aggregators become usable at ingestion time, comment in the lines below:
+      // {
+      //   group: 'first',
+      //   suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
+      // },
+      // {
+      //   group: 'last',
+      //   suggestions: ['longLast', 'doubleLast', 'floatLast'],
+      // },

Review comment:
       We don't usually allow commented out code in java code. Do you think it's better to be consistent in the web-console as well?




----------------------------------------------------------------
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] FrankChen021 commented on pull request #10794: Web console: Remove first / last suggestions

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


   @suneet-s I'm working on #10702. I have finished main coding and unit test cases, and is going to add new IT test cases. But it will take me some time to finish it since I don't get a large chunk of time these days. Once the fix is completed, I will bring back the UI removed by this PR.


----------------------------------------------------------------
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] jihoonson commented on a change in pull request #10794: Web console: Remove first / last suggestions

Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #10794:
URL: https://github.com/apache/druid/pull/10794#discussion_r563920310



##########
File path: web-console/src/druid-models/metric-spec.tsx
##########
@@ -67,14 +67,17 @@ export const METRIC_SPEC_FIELDS: Field<MetricSpec>[] = [
         group: 'max',
         suggestions: ['longMax', 'doubleMax', 'floatMax'],
       },
-      {
-        group: 'first',
-        suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
-      },
-      {
-        group: 'last',
-        suggestions: ['longLast', 'doubleLast', 'floatLast'],
-      },
+      // Do not show first and last aggregators as they can not be used in ingestion specs and this definition is only used in the data loader.
+      // Ref: https://druid.apache.org/docs/latest/querying/aggregations.html#first--last-aggregator
+      // Should the first / last aggregators become usable at ingestion time, comment in the lines below:
+      // {
+      //   group: 'first',
+      //   suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
+      // },
+      // {
+      //   group: 'last',
+      //   suggestions: ['longLast', 'doubleLast', 'floatLast'],
+      // },

Review comment:
       We don't usually allow commented out code in java code. Do you think it's better to be consistent in the web-console as well?




----------------------------------------------------------------
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 a change in pull request #10794: Web console: Remove first / last suggestions

Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10794:
URL: https://github.com/apache/druid/pull/10794#discussion_r563954782



##########
File path: web-console/src/druid-models/metric-spec.tsx
##########
@@ -67,14 +67,17 @@ export const METRIC_SPEC_FIELDS: Field<MetricSpec>[] = [
         group: 'max',
         suggestions: ['longMax', 'doubleMax', 'floatMax'],
       },
-      {
-        group: 'first',
-        suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
-      },
-      {
-        group: 'last',
-        suggestions: ['longLast', 'doubleLast', 'floatLast'],
-      },
+      // Do not show first and last aggregators as they can not be used in ingestion specs and this definition is only used in the data loader.
+      // Ref: https://druid.apache.org/docs/latest/querying/aggregations.html#first--last-aggregator
+      // Should the first / last aggregators become usable at ingestion time, comment in the lines below:
+      // {
+      //   group: 'first',
+      //   suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
+      // },
+      // {
+      //   group: 'last',
+      //   suggestions: ['longLast', 'doubleLast', 'floatLast'],
+      // },

Review comment:
       fixed.




----------------------------------------------------------------
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 a change in pull request #10794: Web console: Remove first / last suggestions

Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #10794:
URL: https://github.com/apache/druid/pull/10794#discussion_r563954782



##########
File path: web-console/src/druid-models/metric-spec.tsx
##########
@@ -67,14 +67,17 @@ export const METRIC_SPEC_FIELDS: Field<MetricSpec>[] = [
         group: 'max',
         suggestions: ['longMax', 'doubleMax', 'floatMax'],
       },
-      {
-        group: 'first',
-        suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
-      },
-      {
-        group: 'last',
-        suggestions: ['longLast', 'doubleLast', 'floatLast'],
-      },
+      // Do not show first and last aggregators as they can not be used in ingestion specs and this definition is only used in the data loader.
+      // Ref: https://druid.apache.org/docs/latest/querying/aggregations.html#first--last-aggregator
+      // Should the first / last aggregators become usable at ingestion time, comment in the lines below:
+      // {
+      //   group: 'first',
+      //   suggestions: ['longFirst', 'doubleFirst', 'floatFirst'],
+      // },
+      // {
+      //   group: 'last',
+      //   suggestions: ['longLast', 'doubleLast', 'floatLast'],
+      // },

Review comment:
       fixed.




----------------------------------------------------------------
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] jihoonson merged pull request #10794: Web console: Remove first / last suggestions

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


   


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