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

Separated nrepl code, added CLI options, added failover #220

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

chrisbetz
Copy link

@chrisbetz chrisbetz commented Jun 30, 2015

Hi,

I'm not sure whether you're interested in this sort of changes or not, so feel free to just drop this PR. But for my use-case (combining GorillaREPL with ApacheSpark) it's really important, so it might be interesting for others. Main rational is I needed to avoid dependency conflicts.

Here's what I did:

  • I separated nrepl code from gorillaREPL webserver to allow for both a standalone version (nrepl + GorillaREPL) and a GorillaREPL-version with the ability to connect to an existing nrepl server.
  • I added CLI options to configure GorillaREPL
  • I added the ability to re-establish a connection from GorillaREPL to the nrepl server. This is still work in progress, as the current REPL-state might get lost and the end-user will not be notified, but it's actually better than breaking the GorillaREPL (in my eyes).

Feel free to take whatever is necessary and wanted and be assured that I find GorillaREPL a really cool project!

Cheers,

Chris


This change is Reviewable

Christian Betz added 2 commits June 29, 2015 13:14
…nrepl server (without relying on "busy" ports for nrepl startup failure)
…This is necessary to "repair" a broken tunnel, or to re-connect to a restarted nrepl server.

However, this is not without problems, as the current REPL state might get lost. At the moment, there's no client-side warning of this happening! That's a big todo!
@JonyEpsilon
Copy link
Owner

Hi Chris,

thanks, this is great :-) That is a feature I'd really like to get into the next release (see #137). I should have time in a few weeks to sit down and put a release together, so will return to this PR then.

Thanks again,

Jony

@chrisbetz
Copy link
Author

Oh, you're welcome. Didn't see that discussion before ;)

Just take your time!

On Tue, Jun 30, 2015 at 12:35 PM, Jony Hudson [email protected]
wrote:

Hi Chris,

thanks, this is great :-) That is a feature I'd really like to get into
the next release (see #137
#137). I should have
time in a few weeks to sit down and put a release together, so will return
to this PR then.

Thanks again,

Jony


Reply to this email directly or view it on GitHub
#220 (comment)
.

@JonyEpsilon
Copy link
Owner

There's a broader discussion that might be of some interest too, thinking about what the relationship should be between nREPL, Gorilla etc. It's at #184 in case you'd like to look :-)

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

Successfully merging this pull request may close these issues.

2 participants