Skip to content

Let the client keep the session open when closing#409

Closed
ralt wants to merge 1 commit into
python-zk:masterfrom
ralt:keep-session
Closed

Let the client keep the session open when closing#409
ralt wants to merge 1 commit into
python-zk:masterfrom
ralt:keep-session

Conversation

@ralt

@ralt ralt commented Oct 20, 2016

Copy link
Copy Markdown
Contributor

The use case is being able to restart a process (e.g. software upgrade)
and not losing the ephemeral nodes. The process can save the session id
and password in a file, reload them at boot time, and load the kazoo
client with the previous session id.

The use case is being able to restart a process (e.g. software upgrade)
and not losing the ephemeral nodes. The process can save the session id
and password in a file, reload them at boot time, and load the kazoo
client with the previous session id.
Comment thread kazoo/client.py
self._stopped.set()
self._queue.append((CloseInstance, None))
if self._keep_session:
self._queue.append((None, None))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be better to use another named constant, instead of None here, something a little more obvious as to what this is doing?

@bbangert

Copy link
Copy Markdown
Member

This will need some tests before it can be merged, ideally something in the docs about it as well. I like the concept over all and think it's good for a new PR with tests/docs.

@jeffwidman

Copy link
Copy Markdown
Member

@bbangert If you think this is a good idea, why did you close the PR?

Normal etiquette on all the OSS projects I'm involved with is to at least ping the original PR author and ask if they're interested in updating it and only after a period of further radio silence to close it for inactivity.

@bbangert

bbangert commented Jun 1, 2017

Copy link
Copy Markdown
Member

Because GitHub provides no usable UX for me to track effectively which PR's had pings, which had no response, and which were updated. When over 50+ PR's are in the backlog, the entire repo unfortunately becomes unusable to an extent. I apologize for the close, as I was using it as a bad UX hack on GH so that I could see updated/new PR's ready for review.

@ralt

ralt commented Jun 1, 2017

Copy link
Copy Markdown
Contributor Author

The close was fine by me. It's not unusual to close a PR that hasn't gotten an update in 6 months, specifically stating that the feature would be accepted if the author is still interested in the involved work.

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.

4 participants