Skip to content

Commit c7dddb1

Browse files
authored
Merge pull request #277 from plotly/andrew/update_last_known_good_chrome
Andrew/update last known good chrome
2 parents c8b22f9 + 75784e0 commit c7dddb1

File tree

10 files changed

+233
-112
lines changed

10 files changed

+233
-112
lines changed

CHANGELOG.txt

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
- Update default chrome from 135.0.7011.0/1418433 to 144.0.7527.0/1544685
2+
- Fix: New chrome takes longer/doesn't populate targets right away, so add a
3+
retry loop to populate targets
4+
- Alter `get_chrome` verbose to print whole JSON
5+
- Change chrome download path to use XDG cache dir
6+
- Don't download chrome if we already have that version: add force argument
17
- Remove unused system inspection code
28
- Add a set of helper functions to await for tab loading and send javascript
39
v1.2.1

ROADMAP.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Roadmap
22

3+
- [ ] Fix up browser deps error (eliminate in-package analysis)
4+
- [ ] Switch to process group and kill that to catch all chromium children
5+
- [ ] Add helpers for running javascript
36
- [ ] Working on better diagnostic information
47
- [ ] Explain to user when their system has security restrictions
58
- [ ] Eliminate synchronous API: it's unused, hard to maintain, and nearly
@@ -15,4 +18,3 @@
1518
- [ ] Do documentation
1619
- [ ] Browser Open/Close Status/PipeCheck status should happen at broker level
1720
- [ ] Broker should probably be opening browser and running watchdog...
18-
- [ ] Add a connect only for websockets

pyproject.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ maintainers = [
3030
]
3131
dependencies = [
3232
"logistro>=2.0.1",
33+
"platformdirs>=4.3.6",
3334
"simplejson>=3.19.3",
3435
]
3536

@@ -124,10 +125,12 @@ sequence = ["test_proc", "test_fn"]
124125
help = "Run all tests quickly"
125126

126127
[tool.poe.tasks.debug-test]
128+
env = { CHOREO_ENABLE_DEBUG="1" }
127129
sequence = ["debug-test_proc", "debug-test_fn"]
128130
help = "Run test by test, slowly, quitting after first error"
129131

130132
[tool.poe.tasks.filter-test]
133+
env = { CHOREO_ENABLE_DEBUG="1" }
131134
cmd = "pytest --log-level=1 -W error -vvvx -rA --capture=no --show-capture=no"
132135
help = "Run any/all tests one by one with basic settings: can include filename and -k filters"
133136

src/choreographer/browser_async.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
from .browsers._interface_type import BrowserImplInterface
3131
from .channels._interface_type import ChannelInterface
3232

33+
_N = MAX_POPULATE_LOOPS = 20
34+
35+
3336
_logger = logistro.getLogger(__name__)
3437

3538
# Since I added locks to pipes, do we need locks here?
@@ -162,9 +165,15 @@ def run() -> subprocess.Popen[bytes] | subprocess.Popen[str]: # depends on args
162165
self._channel.open() # should this and below be in a broker run
163166
_logger.debug("Running read loop")
164167
self._broker.run_read_loop()
165-
_logger.debug("Populating Targets")
166-
await asyncio.sleep(0) # let watchdog start
167-
await self.populate_targets()
168+
await asyncio.sleep(0) # let watchdog start before populate
169+
counter = 0
170+
while not self.get_tab():
171+
_logger.debug("Populating Targets")
172+
await self.populate_targets()
173+
await asyncio.sleep(0.1)
174+
counter += 1
175+
if counter == MAX_POPULATE_LOOPS:
176+
break
168177
except (BrowserClosedError, BrowserFailedError, asyncio.CancelledError) as e:
169178
raise BrowserFailedError(
170179
"The browser seemed to close immediately after starting.",
@@ -345,7 +354,6 @@ async def populate_targets(self) -> None:
345354
raise RuntimeError("Could not get targets") from Exception(
346355
response["error"],
347356
)
348-
349357
for json_response in response["result"]["targetInfos"]:
350358
if (
351359
json_response["type"] == "page"

src/choreographer/cli/_cli_utils.py

Lines changed: 78 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
supported_platform_strings = ["linux64", "win32", "win64", "mac-x64", "mac-arm64"]
2424

2525

26-
def get_google_supported_platform_string() -> tuple[str, str, str, str]:
26+
def get_google_supported_platform_string() -> str | None:
2727
arch_size_detected = "64" if sys.maxsize > 2**32 else "32"
2828
arch_detected = "arm" if platform.processor() == "arm" else "x"
2929

@@ -39,16 +39,17 @@ def get_google_supported_platform_string() -> tuple[str, str, str, str]:
3939
if chrome_platform_detected in supported_platform_strings:
4040
platform_string = chrome_platform_detected
4141

42-
return platform_string, arch_size_detected, platform.processor(), platform.system()
42+
return platform_string
4343

4444

4545
def get_chrome_download_path() -> Path | None:
46-
_chrome_platform_detected, _, _, _ = get_google_supported_platform_string()
46+
_chrome_platform_detected = get_google_supported_platform_string()
4747

4848
if not _chrome_platform_detected:
4949
return None
5050

51-
_default_exe_path = Path()
51+
_default_exe_path = default_download_path
52+
_default_exe_path.mkdir(parents=True, exist_ok=True)
5253

5354
if platform.system().startswith("Linux"):
5455
_default_exe_path = (
@@ -85,47 +86,49 @@ def _extract_member(self, member, targetpath, pwd): # type: ignore [no-untyped-
8586
return path
8687

8788

88-
def get_chrome_sync( # noqa: PLR0912, C901
89+
def get_chrome_sync( # noqa: C901, PLR0912
8990
arch: str | None = None,
9091
i: int | None = None,
9192
path: str | Path = default_download_path,
9293
*,
9394
verbose: bool = False,
95+
force: bool = False,
9496
) -> Path | str:
9597
"""Download chrome synchronously: see `get_chrome()`."""
96-
if not arch:
97-
arch, _, _, _ = get_google_supported_platform_string()
98+
if isinstance(path, str):
99+
path = Path(path)
98100

101+
arch = arch or get_google_supported_platform_string()
99102
if not arch:
100103
raise RuntimeError(
101104
"You must specify an arch, one of: "
102105
f"{', '.join(supported_platform_strings)}. "
103106
f"Detected {arch} is not supported.",
104107
)
105108

106-
if isinstance(path, str):
107-
path = Path(path)
108109
if i:
109110
_logger.info("Loading chrome from list")
111+
raw_json = urllib.request.urlopen( # noqa: S310 audit url for schemes
112+
_chrome_for_testing_url,
113+
).read()
110114
browser_list = json.loads(
111-
urllib.request.urlopen( # noqa: S310 audit url for schemes
112-
_chrome_for_testing_url,
113-
).read(),
115+
raw_json,
114116
)
115117
version_obj = browser_list["versions"][i]
118+
raw_json = json.dumps(version_obj)
116119
else:
117120
_logger.info("Using last known good version of chrome")
118-
with (
121+
raw_json = (
119122
Path(__file__).resolve().parent.parent
120123
/ "resources"
121124
/ "last_known_good_chrome.json"
122-
).open() as f:
123-
version_obj = json.load(f)
124-
if verbose:
125-
print(version_obj["version"]) # noqa: T201 allow print in cli
126-
print(version_obj["revision"]) # noqa: T201 allow print in cli
125+
).read_text()
126+
version_obj = json.loads(
127+
raw_json,
128+
)
129+
version_string = f"{version_obj['version']}\n{version_obj['revision']}"
127130
chromium_sources = version_obj["downloads"]["chrome"]
128-
url = ""
131+
129132
for src in chromium_sources:
130133
if src["platform"] == arch:
131134
url = src["url"]
@@ -137,19 +140,16 @@ def get_chrome_sync( # noqa: PLR0912, C901
137140
f"{arch} is not supported.",
138141
)
139142

140-
if not path.exists():
141-
path.mkdir(parents=True)
142-
filename = path / "chrome.zip"
143-
with urllib.request.urlopen(url) as response, filename.open("wb") as out_file: # noqa: S310 audit url
144-
shutil.copyfileobj(response, out_file)
145-
with _ZipFilePermissions(filename, "r") as zip_ref:
146-
zip_ref.extractall(path)
147-
filename.unlink()
143+
if verbose:
144+
print(raw_json) # noqa: T201 allow print in cli
145+
version_tag = path / "version_tag.txt"
146+
147+
path.mkdir(parents=True, exist_ok=True)
148148

149149
if arch.startswith("linux"):
150-
exe_name = path / f"chrome-{arch}" / "chrome"
150+
exe_path = path / f"chrome-{arch}" / "chrome"
151151
elif arch.startswith("mac"):
152-
exe_name = (
152+
exe_path = (
153153
path
154154
/ f"chrome-{arch}"
155155
/ "Google Chrome for Testing.app"
@@ -158,10 +158,37 @@ def get_chrome_sync( # noqa: PLR0912, C901
158158
/ "Google Chrome for Testing"
159159
)
160160
elif arch.startswith("win"):
161-
exe_name = path / f"chrome-{arch}" / "chrome.exe"
161+
exe_path = path / f"chrome-{arch}" / "chrome.exe"
162162
else:
163163
raise RuntimeError("Couldn't calculate exe_name, unsupported architecture.")
164-
return exe_name
164+
165+
if (
166+
exe_path.exists()
167+
and version_tag.is_file()
168+
and version_tag.read_text() == version_string
169+
and not force
170+
):
171+
return exe_path
172+
else:
173+
if exe_path.exists(): # delete it
174+
if exe_path.is_dir():
175+
shutil.rmtree(exe_path)
176+
else:
177+
exe_path.unlink()
178+
# It really should always be a dir but in testing we fake it
179+
if version_tag.exists(): # delete it
180+
version_tag.unlink()
181+
182+
# Download
183+
zip_path = path / "chrome.zip"
184+
with urllib.request.urlopen(url) as response, zip_path.open("wb") as out_file: # noqa: S310 audit url
185+
shutil.copyfileobj(response, out_file)
186+
with _ZipFilePermissions(zip_path, "r") as zip_ref:
187+
zip_ref.extractall(path)
188+
zip_path.unlink()
189+
version_tag.write_text(version_string)
190+
191+
return exe_path
165192

166193

167194
async def get_chrome(
@@ -170,6 +197,7 @@ async def get_chrome(
170197
path: str | Path = default_download_path,
171198
*,
172199
verbose: bool = False,
200+
force: bool = False,
173201
) -> Path | str:
174202
"""
175203
Download google chrome from google-chrome-for-testing server.
@@ -180,10 +208,18 @@ async def get_chrome(
180208
still in the testing directory.
181209
path: where to download it too (the folder).
182210
verbose: print out version found
211+
force: download chrome again even if already present at that version
183212
184213
"""
185214
loop = asyncio.get_running_loop()
186-
fn = partial(get_chrome_sync, arch=arch, i=i, path=path, verbose=verbose)
215+
fn = partial(
216+
get_chrome_sync,
217+
arch=arch,
218+
i=i,
219+
path=path,
220+
verbose=verbose,
221+
force=force,
222+
)
187223
return await loop.run_in_executor(
188224
executor=None,
189225
func=fn,
@@ -231,12 +267,21 @@ def get_chrome_cli() -> None:
231267
action="store_true",
232268
help="Display found version number if using -i (to stdout)",
233269
)
270+
parser.add_argument(
271+
"-f",
272+
"--force",
273+
dest="force",
274+
action="store_true",
275+
default=False,
276+
help="Force download even if already present.",
277+
)
234278
parser.set_defaults(path=default_download_path)
235279
parser.set_defaults(arch=None)
236280
parser.set_defaults(verbose=False)
237281
parsed = parser.parse_args()
238282
i = parsed.i
239283
arch = parsed.arch
240284
path = Path(parsed.path)
285+
force = parsed.force
241286
verbose = parsed.verbose
242-
print(get_chrome_sync(arch=arch, i=i, path=path, verbose=verbose)) # noqa: T201 allow print in cli
287+
print(get_chrome_sync(arch=arch, i=i, path=path, verbose=verbose, force=force)) # noqa: T201 allow print in cli

src/choreographer/cli/defaults.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,12 @@
22

33
from pathlib import Path
44

5-
default_download_path = Path(__file__).resolve().parent / "browser_exe"
5+
from platformdirs import PlatformDirs
6+
7+
default_download_path = (
8+
Path(
9+
PlatformDirs("choreographer", "plotly").user_data_dir,
10+
)
11+
/ "deps"
12+
)
613
"""The path where we download chrome if no path argument is supplied."""
Lines changed: 1 addition & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,72 +1 @@
1-
{
2-
"version": "135.0.7011.0",
3-
"revision": "1418433",
4-
"downloads": {
5-
"chrome": [
6-
{
7-
"platform": "linux64",
8-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/linux64/chrome-linux64.zip"
9-
},
10-
{
11-
"platform": "mac-arm64",
12-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-arm64/chrome-mac-arm64.zip"
13-
},
14-
{
15-
"platform": "mac-x64",
16-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-x64/chrome-mac-x64.zip"
17-
},
18-
{
19-
"platform": "win32",
20-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win32/chrome-win32.zip"
21-
},
22-
{
23-
"platform": "win64",
24-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win64/chrome-win64.zip"
25-
}
26-
],
27-
"chromedriver": [
28-
{
29-
"platform": "linux64",
30-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/linux64/chromedriver-linux64.zip"
31-
},
32-
{
33-
"platform": "mac-arm64",
34-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-arm64/chromedriver-mac-arm64.zip"
35-
},
36-
{
37-
"platform": "mac-x64",
38-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-x64/chromedriver-mac-x64.zip"
39-
},
40-
{
41-
"platform": "win32",
42-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win32/chromedriver-win32.zip"
43-
},
44-
{
45-
"platform": "win64",
46-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win64/chromedriver-win64.zip"
47-
}
48-
],
49-
"chrome-headless-shell": [
50-
{
51-
"platform": "linux64",
52-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/linux64/chrome-headless-shell-linux64.zip"
53-
},
54-
{
55-
"platform": "mac-arm64",
56-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-arm64/chrome-headless-shell-mac-arm64.zip"
57-
},
58-
{
59-
"platform": "mac-x64",
60-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/mac-x64/chrome-headless-shell-mac-x64.zip"
61-
},
62-
{
63-
"platform": "win32",
64-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win32/chrome-headless-shell-win32.zip"
65-
},
66-
{
67-
"platform": "win64",
68-
"url": "https://storage.googleapis.com/chrome-for-testing-public/135.0.7011.0/win64/chrome-headless-shell-win64.zip"
69-
}
70-
]
71-
}
72-
}
1+
{"version": "144.0.7527.0", "revision": "1544685", "downloads": {"chrome": [{"platform": "linux64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/linux64/chrome-linux64.zip"}, {"platform": "mac-arm64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-arm64/chrome-mac-arm64.zip"}, {"platform": "mac-x64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-x64/chrome-mac-x64.zip"}, {"platform": "win32", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win32/chrome-win32.zip"}, {"platform": "win64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win64/chrome-win64.zip"}], "chromedriver": [{"platform": "linux64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/linux64/chromedriver-linux64.zip"}, {"platform": "mac-arm64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-arm64/chromedriver-mac-arm64.zip"}, {"platform": "mac-x64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-x64/chromedriver-mac-x64.zip"}, {"platform": "win32", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win32/chromedriver-win32.zip"}, {"platform": "win64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win64/chromedriver-win64.zip"}], "chrome-headless-shell": [{"platform": "linux64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/linux64/chrome-headless-shell-linux64.zip"}, {"platform": "mac-arm64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-arm64/chrome-headless-shell-mac-arm64.zip"}, {"platform": "mac-x64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/mac-x64/chrome-headless-shell-mac-x64.zip"}, {"platform": "win32", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win32/chrome-headless-shell-win32.zip"}, {"platform": "win64", "url": "https://storage.googleapis.com/chrome-for-testing-public/144.0.7527.0/win64/chrome-headless-shell-win64.zip"}]}}

0 commit comments

Comments
 (0)