zeppelin
zeppelin copied to clipboard
[ZEPPELIN-5896]feat:add sql debug feature
What is this PR for?
This pull request add sql debug feat into zeppelin
Design Document: https://docs.google.com/document/d/1b0MkUwnPdqXQ3uALpELUUau0D50QEM6z_Xuwrs4GQmA/edit?usp=sharing
What type of PR is it?
Feature
Todos
- [ ] - Task
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-5896
How should this be tested?
- CI
Questions:
- Does the license files need to update? No
- Is there breaking changes for older versions? No
- Does this needs documentation? Yes
Is there a reason why you open so many pull requests with the same subject? You can always change the existing pull request. If everything works with the Maven plugin, the pull request can be shortened to a manageable minimum.
ok,sorry,i am not familiar with it, i will try it next times,thanks for your advices
hi @Reamer is this pr has other problems? i really want to add this feature to zeppelin, could you help me complete it?
hi @Reamer is this pr has other problems? i really want to add this feature to zeppelin, could you help me complete it?
Don't you see the review comments from me?
hi @Reamer is this pr has other problems? i really want to add this feature to zeppelin, could you help me complete it?
Don't you see the review comments from me?
i have changed it according to your advice
I do not see your changes.
Please also remove the default configurations. You can probably also delete most of the files under zeppelin-interpreter/src/main/java/org/apache/zeppelin/antrl4/.
I do not see your changes.
the older version
the latest version
Please also remove the default configurations. You can probably also delete most of the files under
zeppelin-interpreter/src/main/java/org/apache/zeppelin/antrl4/.
Please also remove the default configurations --- this is more detaily?such as give the line of code
hi,if delete the code in zeppelin-interpreter/src/main/java/org/apache/zeppelin/antrl4/ ,the feature cannot run ,really to do it?
I have created https://github.com/zhugezifang/zeppelin/pull/1 for you. Please look over it. The compilation works. I can't make any statement about the functionality.
I have created zhugezifang#1 for you. Please look over it. The compilation works. I can't make any statement about the functionality.
thank you very much for your guidance,i look over it
We need also an integration in the new UI
zeppelin-web-angular.
yes,it also need
We need also an integration in the new UI
zeppelin-web-angular.
hi @Reamer i am not familiar with the new ui of angular , is there a developer familiar with angular in zeppelin community to complete it?
We need also an integration in the new UI
zeppelin-web-angular.hi @Reamer i am not familiar with the new ui of angular , is there a developer familiar with angular in zeppelin community to complete it?
We need also an integration in the new UI
zeppelin-web-angular.
i try to completed it
I found two minor improvements. Can you correct your formatter. For example, Zeppelin uses an indentation of two spaces. Furthermore, the use of antlr4 must be included in the license. Also, it would be nice if you upload screenshots of both frontends in the pull request description. Glad you've been reading up on the new Angular frontend. In my opinion, this one was always easier than the other. But I'm not a frontend developer and can't really evaluate your code there.
ok,thans for your advices,it really help me very much , i learn a lot from processing the problems of this pr , and i completed it
I found two minor improvements. Can you correct your formatter. For example, Zeppelin uses an indentation of two spaces. Furthermore, the use of antlr4 must be included in the license. Also, it would be nice if you upload screenshots of both frontends in the pull request description. Glad you've been reading up on the new Angular frontend. In my opinion, this one was always easier than the other. But I'm not a frontend developer and can't really evaluate your code there.
ok,thans for your advices,it really help me very much , i learn a lot from processing the problems of this pr , and i completed it
@Reamer hi,could you help to review it again?
I found two minor improvements. Can you correct your formatter. For example, Zeppelin uses an indentation of two spaces. Furthermore, the use of antlr4 must be included in the license. Also, it would be nice if you upload screenshots of both frontends in the pull request description. Glad you've been reading up on the new Angular frontend. In my opinion, this one was always easier than the other. But I'm not a frontend developer and can't really evaluate your code there.
@Reamer hi, could you help to review it again? is this pr can be merged?
Thank you for your contribution. I already saw some comments by @Reamer but I also could see some style issue. (Yes, Zeppelin doesn't have very-very strict rules for styling but I think contributors should do their best to keep the style with the current one. Some Java code doesn't match with the current code so I recommend you reformat your code block in your IDE. Could you please do it kindly?
Thank you for your contribution. I already saw some comments by @Reamer but I also could see some style issue. (Yes, Zeppelin doesn't have very-very strict rules for styling but I think contributors should do their best to keep the style with the current one. Some Java code doesn't match with the current code so I recommend you reformat your code block in your IDE. Could you please do it kindly?
no problem,i will keep it ,thanks for your help @Reamer @jongyoul ,it really help me lot
I have added a few comments. Please remember to include images of both front ends in the PullRequest description.
hi @Reamer ,i not do the new ui ,You see, okay? i am really not familiar with the angular
I am very unhappy with the code quality and therefore will not pursue this feature. Another reviewer is welcome to ask my opinion once the code is clean. In my opinion, most of the code belongs in the JDBC interpreter and not in the general Zeppelin interpreter. There should also be an implementation for the new frontend, since the old frontend will be replaced at some point. In the backend will not check for a JDBC paragraph. The check takes place only in the frontend.... @zhugezifang I don't have the time to develop this feature.
I am very sorry, i will improve the code quality, and i will try to completed the new ui , because i am not familiar with angular,it may cost me lot time, but i will try my best to do it. and you really help me a lot to develop this feature, the Limite of my ability,i complete the code which cannot meet your requirements,it really sorry,i try to improve it .At last, Thank you sincerely for your advice very much @Reamer and @jongyoul
I am very unhappy with the code quality and therefore will not pursue this feature. Another reviewer is welcome to ask my opinion once the code is clean. In my opinion, most of the code belongs in the JDBC interpreter and not in the general Zeppelin interpreter. There should also be an implementation for the new frontend, since the old frontend will be replaced at some point. In the backend will not check for a JDBC paragraph. The check takes place only in the frontend.... @zhugezifang I don't have the time to develop this feature.
hi, i try my best to improve the code quality, and completed the new ui ,and add images of both front ends in the PullRequest description
Thank you for your contribution. I already saw some comments by @Reamer but I also could see some style issue. (Yes, Zeppelin doesn't have very-very strict rules for styling but I think contributors should do their best to keep the style with the current one. Some Java code doesn't match with the current code so I recommend you reformat your code block in your IDE. Could you please do it kindly?
hi @jongyoul could you help me to review this pr?
Thank you for your contribution. I already saw some comments by @Reamer but I also could see some style issue. (Yes, Zeppelin doesn't have very-very strict rules for styling but I think contributors should do their best to keep the style with the current one. Some Java code doesn't match with the current code so I recommend you reformat your code block in your IDE. Could you please do it kindly?
hi @jongyoul could you help me to review this pr?
@jongyoul hi
@zhugezifang Hello, sorry for the late reply. I glance at your PR and suggest you make a child interpreter like jdbc.debug instead of adding a menu. The debug paragraph is currently only for JDBC, but the menu would be exposed for all kinds of paragraphs. zeppelin-interpreter package and NotebookServer class affect all of interpreters. If you want to make an interpreter-specific feature, I recommend adding a new sub-interpreter. WDYT?
@zhugezifang Hello, sorry for the late reply. I glance at your PR and suggest you make a child interpreter like
jdbc.debuginstead of adding a menu. The debug paragraph is currently only for JDBC, but the menu would be exposed for all kinds of paragraphs.zeppelin-interpreterpackage andNotebookServerclass affect all of interpreters. If you want to make an interpreter-specific feature, I recommend adding a new sub-interpreter. WDYT?
@jongyoul hi,the menu of sql debug is controlled, it only can be viewed and used in jdbc interpreter, it cannot be viewed in other interpreter
@zhugezifang Hello, sorry for the late reply. I glance at your PR and suggest you make a child interpreter like
jdbc.debuginstead of adding a menu. The debug paragraph is currently only for JDBC, but the menu would be exposed for all kinds of paragraphs.zeppelin-interpreterpackage andNotebookServerclass affect all of interpreters. If you want to make an interpreter-specific feature, I recommend adding a new sub-interpreter. WDYT?
@zhugezifang Hello, sorry for the late reply. I glance at your PR and suggest you make a child interpreter like
jdbc.debuginstead of adding a menu. The debug paragraph is currently only for JDBC, but the menu would be exposed for all kinds of paragraphs.zeppelin-interpreterpackage andNotebookServerclass affect all of interpreters. If you want to make an interpreter-specific feature, I recommend adding a new sub-interpreter. WDYT?@jongyoul hi,the menu of sql debug is controlled, it only can be viewed and used in jdbc interpreter, it cannot be viewed in other interpreter
@jongyoul hi,could help me to review it?
