SeasideSt / Seaside

The framework for developing sophisticated web applications in Smalltalk.
MIT License
519 stars 71 forks source link

Drop Swazoo from Seaside for GemStone? #1222

Closed dalehenrich closed 4 years ago

dalehenrich commented 4 years ago

For 3.6.x support a change needs to be made to the Swazoo2 server code: GsProcess>>terminate will block and ultimately timeout with an error if a socket is waiting on a GsSocket>>accept call ... The code needs to be changed to use GsSocket>>readWillNotBlockWithin: which will not block on #terminate. It's pretty fundamental for a Seaside server process to be terminated (without errors) and it is relatively straightforward to arrange to insert the use of GsSocket>>readWillNotBlockWithin:. The Zinc-based server code used by GsApplicationTools already uses GsSocket>>readWillNotBlockWithin:.

I was in the process of making this change to the Swazoo code, when I realized that the Swazoo2 project is on SmalltalkHub which is now a static site. Before I go off and create github project for Swazoo2, so I can make the code changes or remove the tests, I thought I would check to see how important it is to continue to support the Swazoo2 code?

I am not aware of anyone actively using Swazoo2 with Seaside.

The latest configurations are on SmalltalkHub, and there are alternate Monticello repositories (in order of "freshness"):

Clearly no one else was motivated to save the code, so I'm not sure myself if I should make the effort to create a Swazoo github project.

Right now my inclination is to simply remove the project from the Seaside baseline and see if anyone complains.

If anyone else does care, it is easy enough to add the project back to the baseline and once someone else has established the new repository of record, I will be able to push my changes there ...

What do you guys (@jbrichau and @marschall) think?

jbrichau commented 4 years ago

I have no objection to drop support for it. We have Zinc and FastCGI as possibilities. We can drop Swazoo for Gemstone 3.6 and upwards.

marschall commented 4 years ago

I'm fine with removing it as well.

dalehenrich commented 4 years ago

Just to be clear which one of the following are you guys (@jbrichau @marschall ) agreeing too?

  1. remove Swazoo2 from Seaside for all platforms
  2. remove Swazoo2 from Seaside for gemstone 3.6.x and beyond

At this moment I'm thinking that you are both agreeing to item 2 ... which is more complicated than just ripping out the Swazoo2, so I'm starting to work through the issues, but if you are both thinking item 1 then I will toss whatever work I may have done in favor of a simpler baseline going forward:)

dalehenrich commented 4 years ago

With this commit 9dcf1abb7 dalehenrich/Seaside is ready for testing against 3.6.0 with Swazoo2 removed (earlier test removing Swazz for 3.5.x passed)

marschall commented 4 years ago

I'm fine with both as I haven't seen much/development regarding Swazoo in recent years.

jbrichau commented 4 years ago

I agree. It's unmaintained and if it takes effort to keep supporting it, we should drop it unless someone steps up to support it.

I will thus not object to option 1. No time right now to look at the code, but if option 2 is possible, then I would prefer it. I leave it at your discretion and time.

dalehenrich commented 4 years ago

Thanks guys ... I implemented option2 and tested with 3.5 ... still working on getting to the point where I can test with 3.6:), but I'll leave seaside as it stands with Swazoo not supported for GemSTone 3.6 and beyond ... I'm doing the work over here and will submit a PR when all of the dust (and smoke:) has settled

dalehenrich commented 4 years ago

with commit 7e4c57dd3, swazoo2 has been removed from 3.6.x and beyond

dalehenrich commented 4 years ago

Still need to merge this work into SeasideSt/Seaside (pointy end of issue_1223_swazoo branch) ...