Skip to content

Commit cbb5f14

Browse files
authored
refactor: simplify the plugin detection code, add test coverage (#1769)
We added instrumentation of the GCP vscode extension and jupyter plugin in pandas-gbq and bigquery-magics. In this change we are propagating the improvements there to BigFrames.
1 parent 0629cac commit cbb5f14

File tree

2 files changed

+93
-28
lines changed

2 files changed

+93
-28
lines changed

bigframes/session/environment.py

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright 2023 Google LLC
1+
# Copyright 2025 Google LLC
22
#
33
# Licensed under the Apache License, Version 2.0 (the "License");
44
# you may not use this file except in compliance with the License.
@@ -12,9 +12,14 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
1516
import importlib
1617
import json
1718
import os
19+
import pathlib
20+
21+
Path = pathlib.Path
22+
1823

1924
# The identifier for GCP VS Code extension
2025
# https://cloud.google.com/code/docs/vscode/install
@@ -29,40 +34,36 @@
2934
def _is_vscode_extension_installed(extension_id: str) -> bool:
3035
"""
3136
Checks if a given Visual Studio Code extension is installed.
32-
3337
Args:
3438
extension_id: The ID of the extension (e.g., "ms-python.python").
35-
3639
Returns:
3740
True if the extension is installed, False otherwise.
3841
"""
3942
try:
4043
# Determine the user's VS Code extensions directory.
41-
user_home = os.path.expanduser("~")
42-
if os.name == "nt": # Windows
43-
vscode_extensions_dir = os.path.join(user_home, ".vscode", "extensions")
44-
elif os.name == "posix": # macOS and Linux
45-
vscode_extensions_dir = os.path.join(user_home, ".vscode", "extensions")
46-
else:
47-
raise OSError("Unsupported operating system.")
44+
user_home = Path.home()
45+
vscode_extensions_dir = user_home / ".vscode" / "extensions"
4846

4947
# Check if the extensions directory exists.
50-
if os.path.exists(vscode_extensions_dir):
51-
# Iterate through the subdirectories in the extensions directory.
52-
for item in os.listdir(vscode_extensions_dir):
53-
item_path = os.path.join(vscode_extensions_dir, item)
54-
if os.path.isdir(item_path) and item.startswith(extension_id + "-"):
55-
# Check if the folder starts with the extension ID.
56-
# Further check for manifest file, as a more robust check.
57-
manifest_path = os.path.join(item_path, "package.json")
58-
if os.path.exists(manifest_path):
59-
try:
60-
with open(manifest_path, "r", encoding="utf-8") as f:
61-
json.load(f)
62-
return True
63-
except (FileNotFoundError, json.JSONDecodeError):
64-
# Corrupted or incomplete extension, or manifest missing.
65-
pass
48+
if not vscode_extensions_dir.exists():
49+
return False
50+
51+
# Iterate through the subdirectories in the extensions directory.
52+
extension_dirs = filter(
53+
lambda p: p.is_dir() and p.name.startswith(extension_id + "-"),
54+
vscode_extensions_dir.iterdir(),
55+
)
56+
for extension_dir in extension_dirs:
57+
# As a more robust check, the manifest file must exist.
58+
manifest_path = extension_dir / "package.json"
59+
if not manifest_path.exists() or not manifest_path.is_file():
60+
continue
61+
62+
# Finally, the manifest file must be a valid json
63+
with open(manifest_path, "r", encoding="utf-8") as f:
64+
json.load(f)
65+
66+
return True
6667
except Exception:
6768
pass
6869

@@ -72,10 +73,8 @@ def _is_vscode_extension_installed(extension_id: str) -> bool:
7273
def _is_package_installed(package_name: str) -> bool:
7374
"""
7475
Checks if a Python package is installed.
75-
7676
Args:
7777
package_name: The name of the package to check (e.g., "requests", "numpy").
78-
7978
Returns:
8079
True if the package is installed, False otherwise.
8180
"""

tests/unit/session/test_clients.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
# limitations under the License.
1414

1515
import os
16+
import pathlib
17+
import tempfile
1618
from typing import Optional
1719
import unittest.mock as mock
1820

@@ -155,6 +157,7 @@ def test_user_agent_not_in_vscode(monkeypatch):
155157
monkeypatch_client_constructors(monkeypatch)
156158
provider = create_clients_provider()
157159
assert_clients_wo_user_agent(provider, "vscode")
160+
assert_clients_wo_user_agent(provider, "googlecloudtools.cloudcode")
158161

159162
# We still need to include attribution to bigframes
160163
assert_clients_w_user_agent(provider, f"bigframes/{bigframes.version.__version__}")
@@ -165,16 +168,48 @@ def test_user_agent_in_vscode(monkeypatch):
165168
monkeypatch_client_constructors(monkeypatch)
166169
provider = create_clients_provider()
167170
assert_clients_w_user_agent(provider, "vscode")
171+
assert_clients_wo_user_agent(provider, "googlecloudtools.cloudcode")
168172

169173
# We still need to include attribution to bigframes
170174
assert_clients_w_user_agent(provider, f"bigframes/{bigframes.version.__version__}")
171175

172176

177+
@mock.patch.dict(os.environ, {"VSCODE_PID": "12345"}, clear=True)
178+
def test_user_agent_in_vscode_w_extension(monkeypatch):
179+
monkeypatch_client_constructors(monkeypatch)
180+
181+
with tempfile.TemporaryDirectory() as tmpdir:
182+
user_home = pathlib.Path(tmpdir)
183+
extension_dir = (
184+
user_home / ".vscode" / "extensions" / "googlecloudtools.cloudcode-0.12"
185+
)
186+
extension_config = extension_dir / "package.json"
187+
188+
# originally extension config does not exist
189+
assert not extension_config.exists()
190+
191+
# simulate extension installation by creating extension config on disk
192+
extension_dir.mkdir(parents=True)
193+
with open(extension_config, "w") as f:
194+
f.write("{}")
195+
196+
with mock.patch("pathlib.Path.home", return_value=user_home):
197+
provider = create_clients_provider()
198+
assert_clients_w_user_agent(provider, "vscode")
199+
assert_clients_w_user_agent(provider, "googlecloudtools.cloudcode")
200+
201+
# We still need to include attribution to bigframes
202+
assert_clients_w_user_agent(
203+
provider, f"bigframes/{bigframes.version.__version__}"
204+
)
205+
206+
173207
@mock.patch.dict(os.environ, {}, clear=True)
174208
def test_user_agent_not_in_jupyter(monkeypatch):
175209
monkeypatch_client_constructors(monkeypatch)
176210
provider = create_clients_provider()
177211
assert_clients_wo_user_agent(provider, "jupyter")
212+
assert_clients_wo_user_agent(provider, "bigquery_jupyter_plugin")
178213

179214
# We still need to include attribution to bigframes
180215
assert_clients_w_user_agent(provider, f"bigframes/{bigframes.version.__version__}")
@@ -185,6 +220,37 @@ def test_user_agent_in_jupyter(monkeypatch):
185220
monkeypatch_client_constructors(monkeypatch)
186221
provider = create_clients_provider()
187222
assert_clients_w_user_agent(provider, "jupyter")
223+
assert_clients_wo_user_agent(provider, "bigquery_jupyter_plugin")
188224

189225
# We still need to include attribution to bigframes
190226
assert_clients_w_user_agent(provider, f"bigframes/{bigframes.version.__version__}")
227+
228+
229+
@mock.patch.dict(os.environ, {"JPY_PARENT_PID": "12345"}, clear=True)
230+
def test_user_agent_in_jupyter_with_plugin(monkeypatch):
231+
monkeypatch_client_constructors(monkeypatch)
232+
233+
def custom_import_module_side_effect(name, package=None):
234+
if name == "bigquery_jupyter_plugin":
235+
return mock.MagicMock()
236+
else:
237+
import importlib
238+
239+
return importlib.import_module(name, package)
240+
241+
assert isinstance(
242+
custom_import_module_side_effect("bigquery_jupyter_plugin"), mock.MagicMock
243+
)
244+
assert custom_import_module_side_effect("bigframes") is bigframes
245+
246+
with mock.patch(
247+
"importlib.import_module", side_effect=custom_import_module_side_effect
248+
):
249+
provider = create_clients_provider()
250+
assert_clients_w_user_agent(provider, "jupyter")
251+
assert_clients_w_user_agent(provider, "bigquery_jupyter_plugin")
252+
253+
# We still need to include attribution to bigframes
254+
assert_clients_w_user_agent(
255+
provider, f"bigframes/{bigframes.version.__version__}"
256+
)

0 commit comments

Comments
 (0)