add default port for remote connection #304
Conversation
| @@ -3146,11 +3146,17 @@ | |||
| `run-driver` and `connect-driver` may accept." | |||
| ([type] | |||
| (boot-driver type {})) | |||
There was a problem hiding this comment.
я сейчас посмотрел, мы делаем похожее в create-driver: тоже ищем порт по дефолтам или рандомный. Хотелось бы, что это было только в одном месте.
There was a problem hiding this comment.
или погоди, можно всю эту логику сдвинуть в create-driver, тем более она и так там частично есть.
src/etaoin/api.clj
Outdated
| (let [port (when host | ||
| (or port | ||
| (get-in defaults [type :port]))) | ||
| opt (if port | ||
| (assoc opt :port port) | ||
| opt)] |
There was a problem hiding this comment.
В общем, мне кажется, что пляски вокруг порта лучше сдвинуть в create-driver, чтобы boot-driver состоял только из набора шагов. В boot мы оставим только условие на (run-driver opt)
There was a problem hiding this comment.
да, я тоже видел что в create-driver уже есть логика, но она немного другая
там момент в том, что тогда discover-port нужно убрать или отрефакторить, из create-driver в run-driver
(discover-port type host))
внутри берет дефолтный порт, если он есть и проверяет не занят ли он, и если занят, то берет рандомный
но в случае коннекта порт точно занят, поэтому всегда получим рандомный порт.
т.е. можно вынести из discover-port получение дефолтного порта, и сам discover-port переместить в run-driver, тогда сразу закроем задачу с форком процесса.
There was a problem hiding this comment.
да, тут нужно привести к общему решению. Предлагаю так:
- хост задан (ремоут)
- порт задан -- используем его
- порт не задан -- берем стандартный
- хост не задан (локально)
- порт задан -- берем его
- порт не задан -- берем случайный
There was a problem hiding this comment.
случай с форком: перед запуском драйвера проверяем, что порт (полученный из предыдущего шага) не занят. Если занят, кидаем исключение.
src/etaoin/api.clj
Outdated
| (get-in defaults [type :port])) | ||
| (if port | ||
| port | ||
| (discover-port type local-host))) |
There was a problem hiding this comment.
чтобы не забыть: рандомный порт и удалить discover-port
#295