You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "Reamer (via GitHub)" <gi...@apache.org> on 2024/09/05 10:11:19 UTC
[PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Reamer opened a new pull request, #4813:
URL: https://github.com/apache/zeppelin/pull/4813
### What is this PR for?
This PR removes the raw usage of the AngularObject. Unfortunately, I was not able to get more specific than object.
### What type of PR is it?
- Refactoring
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-6077
### 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? No
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "voidmatcha (via GitHub)" <gi...@apache.org>.
voidmatcha commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-6292928097
@jongyoul Regarding your [comment](https://github.com/apache/zeppelin/pull/4813#issuecomment-6281220650), this PR appears to refactor the server-side binding API for the existing Angular Display functionality, so I don't think it is directly related to the current React migration. I'll take a closer look at compatibility with existing user code when I have time.
Support for `%angular` in React has not been implemented yet, but I'll add it to the notebook migration TODO list. The plan is to keep the Angular UI as the default until the migration is complete, with the React UI available only when explicitly enabled via a flag.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-2352127617
Review required.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-3297958148
Review required.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-5707434057
This pull request has been inactive for over a year. If no further activity occurs within the next 30 days, it will be automatically closed. If you believe this PR is still relevant, please feel free to leave a comment or make an update. Thank you!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "voidmatcha (via GitHub)" <gi...@apache.org>.
voidmatcha commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-6293054036
@jongyoul Thank you for pointing this out. Regarding your [comment](https://github.com/apache/zeppelin/pull/4813#issuecomment-6281220650), this PR appears to refactor the server-side binding API for the existing Angular Display functionality, so I don't expect merging it to directly affect the current React UI. Compatibility with existing notebooks and interpreter code that use the Angular Display APIs still needs a separate review, though. I'll take a closer look at that when I have a chance.
Support for `%angular` in React has not been implemented yet, but I'll add it to the notebook migration TODO list. We plan to keep the Angular UI as the default until the migration is complete, with the React UI available only when explicitly enabled via a flag. With that approach, I believe we can safely preserve the existing user workflow while preparing React support for this functionality.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-6281220650
@voidmatcha Could you please help to check it? It relates to the react ui issue I think.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-6244627088
This pull request has been automatically closed due to prolonged inactivity (over one year without updates). If you feel this was done in error or would like to continue the discussion, feel free to reopen it. Thank you for your contributions!
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#discussion_r1761001476
##########
spark/interpreter/src/main/scala/org/apache/zeppelin/display/angular/AbstractAngularElem.scala:
##########
@@ -134,7 +134,7 @@ abstract class AbstractAngularElem(val interpreterContext: InterpreterContext,
Text(s"${functionName}=${functionName} + 1"),
Null)
- val angularObject = addAngularObject(functionName, 0)
+ val angularObject = addAngularObject(functionName, Int.box(0))
Review Comment:
This is the only place where I am not sure that this is completely correct.
##########
spark/interpreter/src/main/scala/org/apache/zeppelin/display/angular/AbstractAngularElem.scala:
##########
@@ -70,7 +70,7 @@ abstract class AbstractAngularElem(val interpreterContext: InterpreterContext,
* @return
*/
@ZeppelinApi
- def model(name: String, value: Any): AbstractAngularElem = {
+ def model(name: String, value: AnyRef): AbstractAngularElem = {
Review Comment:
AnyRef is the same as object.
https://docs.scala-lang.org/tour/unified-types.html#scala-type-hierarchy
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #4813:
URL: https://github.com/apache/zeppelin/pull/4813#issuecomment-2400156750
Review required.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ZEPPELIN-6077] Remove raw types for AngularObject [zeppelin]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4813: [ZEPPELIN-6077] Remove raw types for AngularObject
URL: https://github.com/apache/zeppelin/pull/4813
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org