Skip to content
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

[WIP][KYUUBI #1352] Introduce FlinkSQLEngine infrastructure #1357

Closed
wants to merge 3 commits into from

Conversation

yanghua
Copy link
Contributor

@yanghua yanghua commented Nov 10, 2021

Why are the changes needed?

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before make a pull request

Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use spaces instead of tabs for indentation

@yaooqinn
Copy link
Member

Awesome~

@yanghua yanghua marked this pull request as draft November 10, 2021 11:30
@yanghua yanghua changed the title [WIP][KYUUBI-1352] Introduce FlinkSQLEngine infrastructure [WIP][KYUUBI #1352] Introduce FlinkSQLEngine infrastructure Nov 10, 2021
Copy link
Member

@pan3793 pan3793 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java code still using TAB for indention, please covert to space, both 2 or 4 are fine

statement: String,
runAsync: Boolean,
queryTimeout: Long): Operation = {
null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of operations are stub now?

@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2021

Codecov Report

Merging #1357 (5d44093) into master (73ac1b6) will decrease coverage by 3.03%.
The diff coverage is 43.01%.

❗ Current head 5d44093 differs from pull request most recent head 936b072. Consider uploading reports for the commit 936b072 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1357      +/-   ##
============================================
- Coverage     59.38%   56.34%   -3.04%     
- Complexity      172      380     +208     
============================================
  Files           237      288      +51     
  Lines         12168    13727    +1559     
  Branches       1492     1551      +59     
============================================
+ Hits           7226     7735     +509     
- Misses         4331     5358    +1027     
- Partials        611      634      +23     
Impacted Files Coverage Δ
.../apache/kyuubi/engine/flink/config/ConfigUtil.java 0.00% <0.00%> (ø)
...uubi/engine/flink/config/entries/CatalogEntry.java 0.00% <0.00%> (ø)
...ubi/engine/flink/config/entries/FunctionEntry.java 0.00% <0.00%> (ø)
...yuubi/engine/flink/config/entries/ModuleEntry.java 0.00% <0.00%> (ø)
...bi/engine/flink/config/entries/SinkTableEntry.java 0.00% <0.00%> (ø)
...ine/flink/config/entries/SourceSinkTableEntry.java 0.00% <0.00%> (ø)
.../engine/flink/config/entries/SourceTableEntry.java 0.00% <0.00%> (ø)
...kyuubi/engine/flink/config/entries/TableEntry.java 0.00% <0.00%> (ø)
...ngine/flink/config/entries/TemporalTableEntry.java 0.00% <0.00%> (ø)
.../kyuubi/engine/flink/config/entries/ViewEntry.java 0.00% <0.00%> (ø)
... and 88 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73ac1b6...936b072. Read the comment docs.

@pan3793 pan3793 added the kind:feature Feature request label Nov 18, 2021
@pan3793 pan3793 added this to the 1.5.0 milestone Nov 18, 2021
@yanghua yanghua force-pushed the KYUUBI-1352 branch 9 times, most recently from 7df07a5 to 3e33762 Compare November 23, 2021 07:50
@yanghua
Copy link
Contributor Author

yanghua commented Dec 28, 2021

merged in #1629 , so close this one, thanks @pan3793

@yanghua yanghua closed this Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature Feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants