-
Notifications
You must be signed in to change notification settings - Fork 334
Run Standard.Google in dual JVM mode
#14040
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
Conversation
corepack pnpm i
corepack pnpm compile
corepack pnpm dev:gui
|
|
I have all the setup to execute the tests in regular JVM mode: now onto running the same tests in dual JVM mode. With bc03db6 there is eleven remaining failures: we are on track to get the dual JVM mode working for |
… JVMs have the same ID
| var emptySheetError = emptySheetType.invokeMember("Error"); | ||
| var errorType = EnsoMeta.getType("Standard.Base.Error", "Error"); | ||
| var error = errorType.invokeMember("throw", emptySheetError); | ||
| throw error.throwException(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- cdbf8fb is a solution to dual exceptions problem, @AdRiley
- rather than throwing Java exception and then trying to convert it to
Empty_Sheet.Error - let's directly throw
Empty_Sheet.Error! - the
getSheetRangemethod returnsTable- as such we need to throw it asPanic(henceerror.throwException()- otherwise just
return error;to propagate the value as (dataflow)Error
- otherwise just
- because we throw
Panicwe havePanic.recoverin the Enso code - if we returned (dataflow)
Error- we could delete the Enso conversion code completely!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- we can go even further with this approach, @jdunkerley
- if we just
return error;then there is no need to do anything special on the Enso side - as de343f3 illustrates, it is possible to remove
Empty_Sheet.handle_java_exception - the only drawback of the de343f3 is the change of function return type to
Object- we need to return either
TableorValueandObjectis the only common superclass
- we need to return either
- the
throw error.throwException()solution workarounds this by returning via an exception (at the cost of necessary handler on the Enso side)
|
Right now we have:
Other than that we are ready to move on. Heuréka! |
| error!("Current API vs Old API: {}", err); | ||
| error!("If you wish to overwrite the current API in the directory {}, run the following command {}, | ||
| error!("If you wish to overwrite the current API in the directory {}, run the following command | ||
| sbt \"runEngineDistribution --no-ir-caches --docs=api --in-project {}\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change, @AdRiley, @GregoryTravis, @jdunkerley, will print:
index 8b63b3d24d..f2c90e184d 100644
--- home/devel/NetBeansProjects/enso/enso/distribution/lib/Standard/Table/0.0.0-dev/docs/api/Errors.md
+++ home/devel/NetBeansProjects/enso/enso/built-distribution/enso-engine-2025.3.1-dev-linux-amd64/enso-2025.3.1-dev/lib/Standard/Table/2025.3.1-dev/docs/api/Errors.md
@@ -43,7 +43,7 @@
- to_display_text self -> Standard.Base.Any.Any
- type Empty_Sheet
- Error
- - handle_java_exception -> Standard.Base.Any.Any
+ - handle_java_exception ~action:Standard.Base.Any.Any -> Standard.Base.Any.Any
- to_display_text self -> Standard.Base.Any.Any
- type Existing_Column
- Error column_name:Standard.Base.Any.Any
ERROR main_internal: enso_build::engine::context: If you wish to overwrite the current API in the directory distribution/lib/Standard/Table/0.0.0-dev/docs/api, run the following command
sbt "runEngineDistribution --no-ir-caches --docs=api --in-project distribution/lib/Standard/Table/0.0.0-dev"
and commit the modified files
ERROR main_internal: enso_build_cli: error=API check failed for library Standard.Table
INFO main_internal: enso_build_cli: close
Error: API check failed for library Standard.Table
in case of API check failure. Which I can then directly copy and run the regenerator:
enso$ sbt "runEngineDistribution --no-ir-caches --docs=api --in-project distribution/lib/Standard/Table/0.0.0-dev"Please note the --no-ir-caches flag added for your convenience as the actionable applied after recent complains.
c1f5e6e to
53f1b1a
Compare
- [Problems upgrading](#14040 (comment)) to most recent develop - likely caused by [allowCoreThreadsTimeout](https://github.com/enso-org/enso/pull/13907/files#r2503655230) - require us to allow access to `Channel` from multiple threads. - technically we would disable `allowCoreThreadsTimeout`, but - we will multi threaded access in the future anyway... - this PR enables such an access. # Important Notes - only master can initialize connection on a new thread - then slave can reply on such a thread or any other thread which was initiated previously by the master - if slave tries to reply on a new thread created in its own JVM, such a message will yield an `IllegalStateException` - the relation between master and slave is **asymmetric** with respect to **threading**
|
| `database-polyglot-root` | ||
| .listFiles("*.jar") | ||
| .map(_.getAbsolutePath()) ++ | ||
| `google-polyglot-root` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The main change of this PR
std-bits/googleare no longer included when buildingensoNI
std-bits/table/src/main/java/org/enso/table/read/ExcelReader.java
Outdated
Show resolved
Hide resolved
|
std-bits/google/src/main/java/org/enso/google/GoogleSheetsForEnso.java
Outdated
Show resolved
Hide resolved
| Convert a Java problem into its Enso equivalent. | ||
| translate_problem p = case p of | ||
| _ : InvalidAggregation -> | ||
| translate_problem p = case p.problemType of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I like this approach, and wonder if each Problem should be responsible for making an Enso object.
distribution/lib/Standard/Google/0.0.0-dev/src/Google_Sheets_Workbook.enso
Show resolved
Hide resolved
std-bits/google/src/main/java/org/enso/google/GoogleSheetsForEnso.java
Outdated
Show resolved
Hide resolved
std-bits/table/src/main/java/org/enso/table/read/ExcelReader.java
Outdated
Show resolved
Hide resolved
std-bits/table/src/main/java/org/enso/table/error/EmptySheetException.java
Show resolved
Hide resolved
|
Re. this comment:
|
jdunkerley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving the libs changes.
I think with your solution we can move in this direction. We can still keep the JUnit tests within the std-bits libraries and keep the tight code cycle I've been there when needed. I'll try out the approach and see how it goes going forward. |



Pull Request Description
Standard.GoogleChecklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
EnsoContext(especially) on "fast path" #14238 fix in