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

Fix Unreliable Hound.Helpers.Session API #123

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

afjackman
Copy link

Changes are summarized in #113

  • Remove ability to name sessions (they have an ID already)
  • Remove tie between sessions and processes
  • Change in_browser_session to perform_in_session
  • Expose active_sessions with function in Hound.Helpers.Session

Changelog will be needed

…ve session name concept to simplify and only use IDs, Expose active sessions
@Kukunin
Copy link

Kukunin commented Nov 5, 2018

Sessions API seems pretty broken, it seems this PR might be a good starting point for rework:

  • Transfer sessions between processes #145 can't transfer session between processes (to use browsers pool, for example)
  • active_sessions returns different info from what can be used
  • hound often leave sessions unclosed (after active work, there were 26 orphan sessions left in chromedriver), haven't looked too close for this.

@danhper
Copy link
Collaborator

danhper commented Nov 10, 2018

Hi @Kukunin, thanks for the comment.
Unfortunately I don't have time to work on it but I would be happy to review PRs.
Thanks.

@Kukunin
Copy link

Kukunin commented Nov 10, 2018

@danhper cool. thanks for the answer. It gives hope that PRs won't be abandoned =) I'll try to send a couple of PRs in the nearest future

@phaer
Copy link

phaer commented Oct 29, 2019

I've tested this PR after rebasing it on the current master branch and it continued to work fine while providing an easier API, in my opinion.

Is there anything blocking this from getting merged?

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.

None yet

5 participants