-
Couldn't load subscription status.
- Fork 209
Async session creation #1552
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
Async session creation #1552
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks useful, and a good start. Thank you for picking this up.
| <td>404 | ||
| <td><code>invalid session creation job id</code> | ||
| <td>Occurs if the given <a>session creation job id</a> is not in the list of <a>session creation jobs</a>, | ||
| or is in the list but has a <a data-lt="session creation status">status</a> of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I can see the reasoning for this, it might be useful to be able to get the cancelled or failed event for inspection.
| </ol> | ||
|
|
||
| <li><p>If <var>found</var> is false, | ||
| return <a>error</a> with <a>error code</a> <a>invalid event id</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's legit to just send the empty list as it just means that there have been no events, right?
|
I can't see a link to tests so adding requires test label |
|
I'd like to propose we start a new spec for this. The concern is that this doesn't seem like something browser vendors are likely to implement directly but something we could standardise with the implementation target being intermediary nodes. I know that's overhead, but unless there's a compelling reason that we actually need this in browser drivers I think it makes sense to seperate out so it's clear who the feature is aimed at. |
|
I'm fine with us speccing this out for intermediate nodes alone, but it should be specced out. |
|
@shs96c I addressed the comments and currently have only 2 remaining issue that I need input for:
What do you all think? |
|
The Browser Testing and Tools Working Group just discussed The full IRC log of that discussion<AutomatedTester> topic: Async Session Creation<AutomatedTester> Github Topic: https://github.com//pull/1552 <AutomatedTester> cb: So the idea is we async session creation job <AutomatedTester> ... that has a defined set of events <AutomatedTester> ... and it has the ability to check that session ID creation <AutomatedTester> ... it has 3 endpoints <AutomatedTester> ... the question is where does the session get the session ID back from <AutomatedTester> ... if we allow a single event or all events <AutomatedTester> q? <simonstewart> q+ <AutomatedTester> jgraham: I think this should be targeted at browser implementations as it seems really good for areas where are a lot of latency like grid or cloud providers <AutomatedTester> ... I also am not sure that I can say that this is hard to justify the time to implement it there <AutomatedTester> ... my suggest is specify in a different spec <AutomatedTester> ... or make it optional for end nodes <AutomatedTester> q? <AutomatedTester> ack jgraham <AutomatedTester> ack simonstewart <AutomatedTester> simonstewart: I think you're right it's important for grid and I see what you mean by resourcing <AutomatedTester> ... in a new spec is overkill <AutomatedTester> ... and we can definitely mark this as a _may_ <AutomatedTester> cb: The main thing for us to make sure that have enough time to get the machines ready at sauce and we don't need to have it like it is <AutomatedTester> jgraham: I think the main use case is definitely yours and I dont think browser vendors should stop you from speccing it <AutomatedTester> Resolution: Add this to webdriver http and mark the new command as a MAY <whimboo> present + <foolip> I have to run, thanks everyone! (son is calling) <AutomatedTester> RRSAgent: bye <RRSAgent> I see no action items <AutomatedTester> oh dear <AutomatedTester> I should ahve force it to make minutes <RRSAgent> logging to https://www.w3.org/2020/10/28-webdriver-irc <AutomatedTester> RRSAgent: make minutes v2 <RRSAgent> I have made the request to generate https://www.w3.org/2020/10/28-webdriver-minutes.html AutomatedTester <AutomatedTester> it's lost the async stuff <jgraham> Nope, all OK <AutomatedTester> oh... just slow <jgraham> RRSAgent: stop <AutomatedTester> yay! <jgraham> Yeah <jgraham> RRSAgent: bye <RRSAgent> I'm staying, jgraham; no access has been specified for the meeting record <jgraham> RRSAgent: make logs public <RRSAgent> I have made the request, jgraham <jgraham> RRSAgent: bye <RRSAgent> I see no action items <AutomatedTester> stupid bot <AutomatedTester> Zakim: bye <jgraham> Zakim, bye <Zakim> leaving. As of this point the attendees have been AutomatedTester, jimevans, shengfa, Honza, cb, jgraham, foolip, john_chen, brwalder, simonstewart, brrian, drousso, mathiasbynens <jgraham> Zakim is the weird one :) |
|
Closing in favor of #1561 |
As a cloud provider starting a WebDriver session can take a while for arbitrary reasons. Long session creation times are problematic for users as request libraries tend to time out after a while. Currently workarounds are used where you would respond with a 302 redirect, redirecting to the same resource. Having a way to asynchronously create a session would help to get rid of such hacky workarounds.
This patch was original authored by @shs96c during TPAC 2018. I would love to finally pick this up and bring it into the spec.
At a high level these would be the step to create an asynchronous session:
[POST] /session/asyncResponse payload would be as follows:
{ "sessionCreationJobId": "6fcd0780e995bd50fedd330cd49d0531", "status": "queueing", "timeout": 600000, "requestedTime": 1601990778990, "events": [], "data": {} }[GET] /session/async/6fcd0780e995bd50fedd330cd49d0531Response payload once job finished loading would be
{ "sessionCreationJobId": "6fcd0780e995bd50fedd330cd49d0531", "status": "created", "timeout": 600000, "requestedTime": 1601990778990, "endedTime": 1601990878990, "events": [{ eventId: "eventId-1" }, { eventId: "eventId-2" }, { eventId: "eventId-3" }, { eventId: "eventId-4" }, ...], "data": {} }[GET] /session/async/6fcd0780e995bd50fedd330cd49d0531/eventId-1Response payload for this request would contain
{ "eventId": "event-1", "time": 1601990778990, "type": "progress", "message": "...", "data": "...", "stacktrace": "..." }Notes from Simon when he drafted this with my comments:
Given that session creation progress is always connected to a point in time I would favor
eventsas representation for it.One timeout for the maximal amount of time it should take to create a job is fine. Overall job timeout is set as session timeout.
IMO attempts can be easier represented in the bindings implementation rather than having to spec this into the protocol.
I propose how it is currently spec'ed with
endedTimeandrequestedTime.Questions:
sessionIdafter the user has fetched a created async job, what am I missing?Preview | Diff