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.

Sign up for GitHub

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

Jump to bottom

ISPN-10776 Server Tasks and Scripts #7445

Merged
merged 1 commit into from Oct 26, 2019
Merged

ISPN-10776 Server Tasks and Scripts #7445

merged 1 commit into from Oct 26, 2019

Conversation

tristantarrant
Copy link
Member

@wburns wburns added the On Hold label Oct 18, 2019
@wburns
Copy link
Member

wburns commented Oct 18, 2019

Just added on hold until #7436 is in.

@tristantarrant tristantarrant added this to the 10.0.0.Final milestone Oct 23, 2019
@ryanemerson
Copy link
Contributor

@tristantarrant Are the AuthorizationLDAPIT failures related? I haven't seen those before.

@tristantarrant
Copy link
Member Author

They are caused by the graceful shutdown commits. I have fixed it in the other PR and I'll rebase this once that is in

@tristantarrant tristantarrant force-pushed the ISPN-10776/server_scripting branch 2 times, most recently from 535a7a6 to eb44842 Compare October 24, 2019 13:32
@tristantarrant
Copy link
Member Author

This is now ready

@ryanemerson
Copy link
Contributor

@tristantarrant I'm looking at the code now, but can you rebase on the latest master to make sure this is Ok with all the marshalling changes on the server

@ryanemerson ryanemerson self-requested a review October 24, 2019 15:57
@tristantarrant
Copy link
Member Author

Done @ryanemerson let's wait for CI

wburns
wburns reviewed Oct 24, 2019
View reviewed changes
server/runtime/src/main/java/org/infinispan/server/Extensions.java Outdated Show resolved Hide resolved
server/runtime/src/test/resources/scripts/stream_serverTask.js
@@ -0,0 +1,30 @@
// mode=local,language=javascript
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where this script is invoked? I didn't think serializable stuff worked in js scripts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I carried over all the scripts from the old testsuite. I have an idea on how to fix this in 10.1.

Copy link
Member

Choose a reason for hiding this comment

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

That would be pretty neat! 💯

@ryanemerson
Copy link
Contributor

It seems like the AuthorizationLDAPIT failures are back 🙁

@ryanemerson
Copy link
Contributor

The ScriptingTasks failure will occur until there is native support for Map in ProtoStream. Until then, I think the JavaSerializationMarshaller will have to be used.

@tristantarrant
Copy link
Member Author

AuthorizationLDAPIT fixed (let's wait for CI though)

@tristantarrant
Copy link
Member Author

tristantarrant commented Oct 24, 2019 •
edited

@ryanemerson @anistor how much work is map support ? Could we convert it to a repeatead pair type ?

@anistor
Copy link
Member

anistor commented Oct 24, 2019

@ryanemerson @anistor how much work is map support ? Could we convert it to a repeatead pair type ?

you could. but there are plenty ways to skin a cat. please do it in a way that is forward compatible.

@tristantarrant
Copy link
Member Author

This is good to merge

@tristantarrant tristantarrant merged commit b7ba7c9 into infinispan:master Oct 26, 2019
@tristantarrant tristantarrant deleted the ISPN-10776/server_scripting branch January 21, 2020 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wburns wburns wburns left review comments

@ryanemerson ryanemerson ryanemerson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
10.0.0.Final
4 participants
@tristantarrant @wburns @ryanemerson @anistor