npawar commented on pull request #6296: URL: https://github.com/apache/incubator-pinot/pull/6296#issuecomment-734504185
Thanks @shahsank3t , this looks great! I’ve listed some of the things that didn’t work for me/ would be nice to change: 1. Can we hide Multi value dropdown for METRIC and DATETIME? 2. In Add Schema - for the mandatory fields, can we have some validation on UI itself? For example, if someone tries to save a Schema without entering a SchemaName or a field’s dataType, currently a call is made to the backend, which fails, and that message is shown back on the UI. Instead, can the UI itself flag it, just by highlighting the incomplete field (and a message, please enter all fields). I’ve seen this done commonly on UIs. 3. Can we have separate starter json template for OFFLINE and REALTIME? In OFFLINE, we don’t want to show the streamConfigs, aggregateMetrics, tagOverrideConfig, peerSegmentDownloadSchema, completionConfig. Similarly, in REALTIME we don’t want to show segmentPushType, segmentPushFrequency and instanceAssignmentConfigMap 4. If someone Enables partitioning, all the columns that show up should be marked mandatory. 5. Same as 4 for Enable Replica Groups 6. TransformConfig did not get set in the IngestionConfig when I added transform functions . I think it is getting created inside filterConfig (instead of outside under ingestionConfig) 7. PartitioningConfig is not getting set. Seeing an error. The json looks incorrect . I’ve added some comments in the doc about it 8. ReplicaGroupConfig is also not getting set. Seeing an error. The json I had provided is incomplete. Added the extra part in the doc. 9. In case of 7 & 8, the error is spilling out of the screen. 10. In REALTIME table, there is an extra blob of “streamConfigs” at the end of the json. 11. In REALTIME table, the changes I made to the stream config properties did not get reflected in the json (e.g. topic name, broker url, or editing the flush threshold time, or removing/adding any property). It is all getting set in that extra blob, as reported in 10. 12. I couldn’t create a REALTIME table. I think it’s because of the properties "stream.kafka.decoder.prop.schema.registry.schema.name" and "stream.kafka.decoder.prop.schema.registry.rest.url". Please remove them from the list. 13. Mark some of the stream config properties as Mandatory. I’ve updated the doc for which ones. 14. In case of REALTIME, when someone enables the “Replica Group”, don’t add the instanceAssignmentConfigMap json. It’s creating an incorrect table config which does not get saved. Either remove it (preferable), or just put null. ---------------------------------------------------------------- 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: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
