HUE-9481 [ui] Create Query Viz component

Review Request #15453 — Created Sept. 21, 2020 and updated

Sreenath
hue
master
HUE-9481
hue
johan, romain
commit b1748a1d5c10432858c5e2787a1a73d50e2afd96
Author: sreenaths <sree@apache.org>
Date:   Mon Sep 21 12:50:25 2020 +0530

    HUE-9481 [ui] Create Query Viz component

:000000 100644 0000000000 024f92f4e3 A	desktop/core/src/desktop/js/components/queryViz/index.vue
:000000 100644 0000000000 880165c53d A	desktop/core/src/desktop/js/components/queryViz/libs/parser-interfaces.ts
:000000 100644 0000000000 d92098b0dd A	desktop/core/src/desktop/js/components/queryViz/libs/parser-service.ts
:000000 100644 0000000000 15fd5b0e8d A	desktop/core/src/desktop/js/components/queryViz/query-viz.scss
:000000 100644 0000000000 affe1e4fd2 A	desktop/core/src/desktop/js/components/queryViz/webcomp.ts
:100644 100644 e930eb820b 47c40642bc M	package-lock.json
:100644 100644 6fedcdbce5 a0caca3403 M	package.json
:000000 100644 0000000000 77a639cc5f A	tools/examples/components/query-viz-demo/index.html
:000000 100644 0000000000 4fdd40816b A	tools/examples/components/query-viz-demo/query-viz.js
:000000 100644 0000000000 e8c480411d A	tools/examples/components/query-viz-demo/styles.css
  • Created demo app
  • Manually tested
  • 20
  • 0
  • 0
  • 0
  • 20
Description From Last Updated
Where does this go in Hue? I can't find a page/component where it's used. johan johan
Package.json is missing from the review. johan johan
Drop the "I", it doesn't add any value and we don't prefix other types. johan johan
ParsedLocation already in parserTypeDefs.ts johan johan
Nit: Split in two lines @Prop() defaultDb: ... johan johan
"any" won't pass the linter. Please define. johan johan
Nit: const entities... johan johan
Nit: entities.push(...parserQuery.tables.values()) johan johan
Same johan johan
Use from parserTypeDefs.ts and js/parse/types.ts instead. johan johan
Nit: use the sqlParserRepository instead. i.e. sqlParserRepository.getSyntaxParser(dialect)/getAutocompleter(dialect) johan johan
ParserService is very generic. Can you think of a better name? johan johan
extractLocations? Also "any" won't pass the linter. johan johan
Drop the log. Did you run the linter? johan johan
No need for the type defs as it'll pick up the return types from the functions. johan johan
Why is this needed? johan johan
any -> IdentifierLocation johan johan
Nit: any. johan johan
Why is this needed? johan johan
Nit: any johan johan
Sreenath
Review request changed

Summary:

-HUE-9481 [ui] Create Query Viz component - WIP
+HUE-9481 [ui] Create Query Viz component
romain
  1. Nice!

    LGTM

    Johan knows more, on my side just wondering (in follow-up commits):
    - Docs? https://docs.gethue.com/developer/components/
    - Plug-it to Hue Editor? (config flag + below the explain button for example)

  2. 
      
johan
  1. 
      
  2. Where does this go in Hue? I can't find a page/component where it's used.

    1. It's still under development. Once it become stable we could add it in the editor page.

  3. Package.json is missing from the review.

    1. Guess it's a RBTool thingy, rebasing and adding.

  4. Drop the "I", it doesn't add any value and we don't prefix other types.

  5. ParsedLocation already in parserTypeDefs.ts

  6. Nit: Split in two lines

    @Prop()
    defaultDb: ...

  7. "any" won't pass the linter. Please define.

  8. Nit: const entities...

  9. Nit: entities.push(...parserQuery.tables.values())

  10. desktop/core/src/desktop/js/components/queryViz/libs/parser-interfaces.ts (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     

    Use from parserTypeDefs.ts and js/parse/types.ts instead.

  11. Nit: use the sqlParserRepository instead. i.e.

    sqlParserRepository.getSyntaxParser(dialect)/getAutocompleter(dialect)

  12. ParserService is very generic. Can you think of a better name?

  13. extractLocations? Also "any" won't pass the linter.

  14. Drop the log. Did you run the linter?

  15. desktop/core/src/desktop/js/components/queryViz/libs/parser-service.ts (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     

    No need for the type defs as it'll pick up the return types from the functions.

  16. desktop/core/src/desktop/js/components/queryViz/libs/parser-service.ts (Diff revision 1)
     
     
     
     
     
     
     
     
     

    Why is this needed?

  17. desktop/core/src/desktop/js/components/queryViz/libs/parser-service.ts (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     

    Why is this needed?

    1. It's needed as the HiveParser isn't returning expressions.
      So for the Hackathon the expressions were parsed using "mathjs" package. This converts Hive expressions into a mathjs friendly format.

  18. 
      
Loading...