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

dont fail on additional json payload like user_variables #440

Closed
maxandersen opened this issue Nov 23, 2023 · 7 comments
Closed

dont fail on additional json payload like user_variables #440

maxandersen opened this issue Nov 23, 2023 · 7 comments

Comments

@maxandersen
Copy link

older kernels like the ones used in google colab sends "user_variables" element.

This makes kotlin kernel fail to run.

Here is log from google colab:


at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:24)
--
Nov 24, 2023, 12:33:33 AM | WARNING | Current input: {"allow_stdin":false,"code":"","silent":true,"store_history":false,"user_expressions":{},"user_variables":[]}

could we make the json parsing more lenient and ignore what it does not know?

@maxandersen
Copy link
Author

basically have Json { ignoreUnknownKeys = true } whereever it is messages are read.

@ileasile
Copy link
Contributor

Hi! Thanks for pointing this out! Maybe just adding a missing field to the scheme is a better solution? Idea was to keep protocol strict. Will it be possible for you to verify the fix in Google Colab if I prepare a new kernel version?

@maxandersen
Copy link
Author

Yes, that would be great! happy to try it out as long as I can get some Maven coordinates to test :)

ileasile added a commit that referenced this issue Nov 25, 2023
Add `user_variables` deprecated but still used field to ExecuteResult message. It is still used in Google Colab

Perform minor refactorings of messaging subsystem
@ileasile
Copy link
Contributor

@maxandersen you can take version 0.12.0-93 to test (or 0.12.0.93 if you're using python dev packages)

@maxandersen
Copy link
Author

it works!

open this gist with google colab and it works.

note: %kotlin-statistics does not work in that notebook but seems to be a jitpack/kotlin kernel issue rather than colab afaics.

@maxandersen
Copy link
Author

note: I've updated the install-kernel script to have the proxy no longer use "ipc" in name, but more that the core kernel will have "-tcp" added in the end. makes the experience on colab consistent with how it works on all the others.

@maxandersen
Copy link
Author

Thanks for fixing that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants