|
| 1 | ++++ |
| 2 | +Description = "Public Project Meeting - February 25, 2021" |
| 3 | +Title = "Public Project Meeting - February 25, 2021" |
| 4 | +Date = 2021-02-25 |
| 5 | +Author = "Sri Harsha" |
| 6 | +AuthorLink = "https://twitter.com/sri_harsha509" |
| 7 | +tags = ["slack","meeting","tlc"] |
| 8 | +categories = ["general","governance"] |
| 9 | ++++ |
| 10 | + |
| 11 | +Continuing the series of bi-weekly public project meetings, here is the |
| 12 | +timeline of the meeting held on February 25, 2021,5:30 PM CET. (Below times are on IST) |
| 13 | + |
| 14 | +Meetings are held on the `#selenium-tlc` channel on [Selenium Slack](https://seleniumhq.slack.com/join/shared_invite/enQtODAwOTUzOTM5OTEwLTZjZjgzN2ExOTBmZGE0NjkwYzA2Nzc0MjczMGYwYjdiNGQ5YjI0ZjdjYjFhMjVlMjFkZWJmNDYyMmU1OTYyM2Y). |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +_Diego Molina 9:54 PM_ |
| 19 | + |
| 20 | +@titusfortner @barancev @harsha509 @luke @jimevans @simonstewart |
| 21 | + |
| 22 | +_Luke Hill 9:54 PM_ |
| 23 | + |
| 24 | +Yip. I'm here if needed. |
| 25 | + |
| 26 | +_Diego Molina 9:55 PM_ |
| 27 | + |
| 28 | +The only topic I have today, to make it a short meeting, is: “what is needed for beta 2” |
| 29 | +Feel free to add/propose any other topics |
| 30 | + |
| 31 | +_Luke Hill 9:59 PM_ |
| 32 | + |
| 33 | +Alongside that, do we have a rough idea of what would enable us to release a 4.0.0 proper (i.e. what milestones we need to hit) |
| 34 | + |
| 35 | +_Diego Molina 10:01 PM_ |
| 36 | + |
| 37 | +Let’s wait a few minutes to see if we have enough people to start the meeting |
| 38 | + |
| 39 | +_Titus Fortner 10:04 PM_ |
| 40 | + |
| 41 | +I have a couple minor things I want to do before beta2, but the sooner the better for me :) |
| 42 | + |
| 43 | +_Simon Stewart 10:04 PM_ |
| 44 | + |
| 45 | +I'm in a planning meeting, but I'll follow along slowly |
| 46 | + |
| 47 | +_Titus Fortner 10:05 PM_ |
| 48 | + |
| 49 | +(Element#dom_attribute is the main one I want to finish). I think Java, C# both have that one already |
| 50 | + |
| 51 | +_Diego Molina 10:05 PM_ |
| 52 | + |
| 53 | +Ok, then let’s start with the ones who are around. Only one topic so far: “what is needed for beta 2” |
| 54 | + |
| 55 | +What comes to my head: fixing the leak :slightly_smiling_face: |
| 56 | + |
| 57 | +_Simon Stewart 10:05 PM_ |
| 58 | + |
| 59 | +Crush the leaks. Ensure CDP on Grid to a Docker instance works |
| 60 | + |
| 61 | +_Diego Molina 10:06 PM_ |
| 62 | + |
| 63 | +Regarding the leak, after reading more about the AsyncHttpClient issues and their google group… |
| 64 | + |
| 65 | +_10:07_ |
| 66 | + |
| 67 | +they mention that each AsyncHttpClient instance has its own pool |
| 68 | + |
| 69 | +_Diego Molina 10:07 PM_ |
| 70 | + |
| 71 | +which is why they recommend to have one single instance of it |
| 72 | + |
| 73 | +_Puja Jagani_ |
| 74 | + |
| 75 | +After you pointed out the potential problem area, I attempted to use single instance of the AsyncHttpClient today and still saw the leak :confused: Will try and dig deeper tomorrow. Let me know if you want me to try something in particular. |
| 76 | + |
| 77 | +_Diego Molina_ |
| 78 | + |
| 79 | +One single instance for the whole distributor? |
| 80 | + |
| 81 | +_Puja Jagani_ |
| 82 | + |
| 83 | +One single instance as we had earlier, similar to https://github.com/SeleniumHQ/selenium/blob/selenium-4.0.0-alpha-7/java/client/src/org/openqa/selenium/remote/http/netty/NettyClient.java#L41 |
| 84 | +java/client/src/org/openqa/selenium/remote/http/netty/NettyClient.java:41 |
| 85 | +private static final AsyncHttpClient httpClient = |
| 86 | +<https://github.com/SeleniumHQ/selenium|SeleniumHQ/selenium>SeleniumHQ/selenium | Added by GitHub |
| 87 | + |
| 88 | +_Puja Jagani_ |
| 89 | + |
| 90 | +This was just to narrow down the problem area. It was something I was trying to eliminate if creating a new instance is the problem or the way we handle the response (blocking bit you mentioned earlier) |
| 91 | + |
| 92 | +_Puja Jagani_ |
| 93 | + |
| 94 | +Distributor heap with single instance :see_no_evil: |
| 95 | + |
| 96 | +_Diego Molina 10:08 PM_ |
| 97 | + |
| 98 | +additionally you can limit the size of the pool |
| 99 | + |
| 100 | +so I am planning to tweak the client configuration and see if that helps |
| 101 | + |
| 102 | +_10:09_ |
| 103 | + |
| 104 | +sadly, the docs for AsyncHttpClient are inexistent, but it seems like a robust implementation |
| 105 | +it is used for Gatling |
| 106 | + |
| 107 | +_10:09_ |
| 108 | + |
| 109 | +(a loat test tool) |
| 110 | + |
| 111 | +_10:10_ |
| 112 | + |
| 113 | +load* |
| 114 | + |
| 115 | +_David Burns 10:12 PM_ |
| 116 | + |
| 117 | +@simonstewart what's left, assuming its everything, to "Ensure CDP on Grid to a Docker instance works" |
| 118 | + |
| 119 | +_Simon Stewart 10:12 PM_ |
| 120 | + |
| 121 | +(In a meeting) |
| 122 | + |
| 123 | +_Titus Fortner 10:13 PM_ |
| 124 | + |
| 125 | +each of the bindings need to be able to implement that part (getting debugger address from caps, etc) |
| 126 | + |
| 127 | +_David Burns 10:13 PM_ |
| 128 | + |
| 129 | +@titusfortner it's done? |
| 130 | + |
| 131 | +_10:14_ |
| 132 | + |
| 133 | +it gets it from se:options |
| 134 | + |
| 135 | +_Titus Fortner 10:14 PM_ |
| 136 | + |
| 137 | +I don't think he's done yet with the first part |
| 138 | + |
| 139 | +he's changing that |
| 140 | + |
| 141 | +_Simon Stewart 10:14 PM_ |
| 142 | + |
| 143 | +se:cdp now |
| 144 | +I updated the bindings that use it |
| 145 | + |
| 146 | +_David Burns 10:14 PM_ |
| 147 | + |
| 148 | +@simonstewart touched my code... filthy java person in my python code |
| 149 | + |
| 150 | +_Simon Stewart 10:14 PM_ |
| 151 | + |
| 152 | +The "CDP in Grid" stuff will add se:cdpVersion |
| 153 | + |
| 154 | +_Luke Hill 10:14 PM_ |
| 155 | + |
| 156 | +One of the new things I noticed in ruby (Not sure if it's relevant across the board). Was the new chrome cdp stuff needs to be able to work in local/remote instances. |
| 157 | + |
| 158 | +_Simon Stewart 10:14 PM_ |
| 159 | + |
| 160 | +"My precious...." |
| 161 | + |
| 162 | +_David Burns 10:14 PM_ |
| 163 | + |
| 164 | +:stuck_out_tongue: |
| 165 | + |
| 166 | +_Simon Stewart 10:15 PM_ |
| 167 | + |
| 168 | +@luke that's what the se:cdp capability allows |
| 169 | + |
| 170 | +_Luke Hill 10:15 PM_ |
| 171 | + |
| 172 | +One step ahead of me :smile: |
| 173 | + |
| 174 | +_Simon Stewart 10:15 PM_ |
| 175 | + |
| 176 | +The Selenium Server knows how to forward CDP traffic |
| 177 | + |
| 178 | +_Titus Fortner 10:15 PM_ |
| 179 | + |
| 180 | +yeah, Ruby code has never actually allowed users to benefit from browser specific functionality in Remote WebDriver because of subclassing blah blah |
| 181 | + |
| 182 | +_Luke Hill 10:15 PM_ |
| 183 | + |
| 184 | +So check back again in beta2 is the answer basically. |
| 185 | + |
| 186 | +_Titus Fortner 10:15 PM_ |
| 187 | + |
| 188 | +I have a PR to address it |
| 189 | + |
| 190 | +_Simon Stewart 10:15 PM_ |
| 191 | + |
| 192 | +I'm going to make the local drivers also set se:cdp |
| 193 | + |
| 194 | +_Titus Fortner 10:16 PM_ |
| 195 | + |
| 196 | +With that PR, the Ruby CDP code works with server only if it is on localhost, so we'll also have to update to what Simon has done |
| 197 | + |
| 198 | +_Luke Hill 10:16 PM_ |
| 199 | + |
| 200 | +We "know" what we need to do. Which is half the battle. |
| 201 | + |
| 202 | +_Titus Fortner 10:17 PM_ |
| 203 | + |
| 204 | +With @p0deje been gone for a while and @twalpole being super busy it's just a matter of bandwidth. :) |
| 205 | + |
| 206 | +well, partly, I only know "ish" |
| 207 | + |
| 208 | +_Diego Molina 10:17 PM_ |
| 209 | + |
| 210 | +it’s the chance for @luke to do some commits :slightly_smiling_face: |
| 211 | + |
| 212 | +_Luke Hill 10:18 PM_ |
| 213 | + |
| 214 | +I can try help where possible. But I'm not quite on their standard. |
| 215 | + |
| 216 | +_Titus Fortner 10:18 PM_ |
| 217 | + |
| 218 | +I'd also like to figure out how to do a different gem publishing for CDP versions |
| 219 | + |
| 220 | +_10:19_ |
| 221 | + |
| 222 | +it's more important that we be able to update the CDP versions to match the browser versions than Selenium methods to CDP methods, so I'd like to be able to release the artifacts independently, and let users toggle it somehow. |
| 223 | + |
| 224 | +_Luke Hill 10:19 PM_ |
| 225 | + |
| 226 | +By Easter I will hopefully be in somewhere more stable again either renting or buying my first place. So yeh should be able to do more in evenings. This is going to be my fifth property move in just over 2 years |
| 227 | + |
| 228 | +_Titus Fortner 10:22 PM_ |
| 229 | + |
| 230 | +How much work is there still in the leak investigation? (I ask because I literally have no idea what all is involved) |
| 231 | + |
| 232 | +_Diego Molina 10:23 PM_ |
| 233 | + |
| 234 | +Not sure, I’ve invested this week and at least I’ve pinned it down to the Distributor |
| 235 | + |
| 236 | +_10:24_ |
| 237 | + |
| 238 | +It eats memory over time when new tests are executed, and never returns it |
| 239 | + |
| 240 | +I am now checking if the problem is really related to the AsyncHttpClient |
| 241 | + |
| 242 | +_10:25_ |
| 243 | + |
| 244 | +well, more related to the way we use it |
| 245 | + |
| 246 | +_David Burns 10:25 PM_ |
| 247 | + |
| 248 | +@diemol @Puja Jagani knows this is a priority so feel free to rope her in :slightly_smiling_face: |
| 249 | + |
| 250 | +_Diego Molina 10:26 PM_ |
| 251 | + |
| 252 | +they recommend to have a single instance, and in the Distributor we have one instance per registered node, and one instance to do the health checks, and another one that I have not pinned down to see what it is |
| 253 | + |
| 254 | +_Simon Stewart 10:30 PM_ |
| 255 | + |
| 256 | +Because each instance needs its own config |
| 257 | + |
| 258 | +_10:31_ |
| 259 | + |
| 260 | +If we could change config per request, we'd be fine |
| 261 | + |
| 262 | +_Diego Molina 10:31 PM_ |
| 263 | + |
| 264 | +they have a different base url, right? |
| 265 | + |
| 266 | +_Simon Stewart 10:31 PM_ |
| 267 | + |
| 268 | +Right |
| 269 | + |
| 270 | +_10:32_ |
| 271 | + |
| 272 | +And possibly time outs |
| 273 | + |
| 274 | +Anything on the config object |
| 275 | + |
| 276 | +_Diego Molina 10:33 PM_ |
| 277 | + |
| 278 | +I did not see different timeouts, but we will need them when we allow users to configure the timeout in Grid |
| 279 | + |
| 280 | +_Puja Jagani_ |
| 281 | + |
| 282 | +I think the RequestBuilder allows to set timeouts per request. |
| 283 | + |
| 284 | +_Diego Molina 10:34 PM_ |
| 285 | + |
| 286 | +so, hm, interesting… |
| 287 | +now sure what to do now :slightly_smiling_face: |
| 288 | + |
| 289 | +_Simon Stewart 10:35 PM_ |
| 290 | + |
| 291 | +Once again, I shall mutter about writing our own http client based on netty, starting from the one we have for domain sockets |
0 commit comments