Skip to content

Commit

Permalink
Merge pull request #297 from krassowski/avoid-importlib-reload
Browse files Browse the repository at this point in the history
Use `importlib.utils.find_spec`, allow to cache results
  • Loading branch information
ryantam626 committed Mar 22, 2023
2 parents ace5fb4 + 9382524 commit a8a063e
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 40 deletions.
15 changes: 9 additions & 6 deletions jupyterlab_code_formatter/formatters.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@
from functools import wraps
from typing import List, Type

import pkg_resources

try:
import rpy2
import rpy2.robjects
except ImportError:
pass
from packaging import version
from functools import cache


logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -59,6 +59,11 @@ def importable(self) -> bool:
def format_code(self, code: str, notebook: bool, **options) -> str:
pass

@property
@cache
def cached_importable(self) -> bool:
return self.importable


class BaseLineEscaper(abc.ABC):
"""A base class for defining how to escape certain sequence of text to avoid formatting."""
Expand Down Expand Up @@ -219,10 +224,8 @@ def wrapped(self, code: str, notebook: bool, **options) -> str:


def is_importable(pkg_name: str) -> bool:
# Need to reload for packages are installed/uninstalled after JupyterLab started
importlib.reload(pkg_resources)

return pkg_name in {pkg.key for pkg in pkg_resources.working_set}
# find_spec will check for packages installed/uninstalled after JupyterLab started
return importlib.util.find_spec(pkg_name) is not None


def command_exist(name: str) -> bool:
Expand Down
14 changes: 11 additions & 3 deletions jupyterlab_code_formatter/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,15 @@ def get(self) -> None:
if self.get_query_argument(
"bypassVersionCheck", default=None
) is not None or check_plugin_version(self):
use_cache = self.get_query_argument("cached", default=None)
self.finish(
json.dumps(
{
"formatters": {
name: {
"enabled": formatter.importable,
"enabled": formatter.cached_importable
if use_cache
else formatter.importable,
"label": formatter.label,
}
for name, formatter in SERVER_FORMATTERS.items()
Expand All @@ -92,8 +95,13 @@ def post(self) -> None:
) is not None or check_plugin_version(self):
data = json.loads(self.request.body.decode("utf-8"))
formatter_instance = SERVER_FORMATTERS.get(data["formatter"])
use_cache = self.get_query_argument("cached", default=None)

if formatter_instance is None or not formatter_instance.importable:
if formatter_instance is None or not (
formatter_instance.cached_importable
if use_cache
else formatter_instance.importable
):
self.set_status(404, f"Formatter {data['formatter']} not found!")
self.finish()
else:
Expand All @@ -116,7 +124,7 @@ def post(self) -> None:

class VersionAPIHandler(APIHandler):
def get(self) -> None:
"""Show what version is this server plguin on."""
"""Show what version is this server plugin on."""
self.finish(
json.dumps(
{
Expand Down
9 changes: 9 additions & 0 deletions schema/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,9 @@
"additionalProperties": false,
"type": "boolean"
},
"cacheFormatters": {
"type": "boolean"
},
"suppressFormatterErrors": {
"additionalProperties": false,
"type": "boolean"
Expand Down Expand Up @@ -367,6 +370,12 @@
"$ref": "#/definitions/formatOnSave",
"default": false
},
"cacheFormatters": {
"title": "Cache formatters",
"description": "Cache formatters on server for better performance (but will not detected newly installed/uninstalled formatters).",
"$ref": "#/definitions/cacheFormatters",
"default": false
},
"astyle": {
"title": "AStyle Config",
"description": "Command line options to be passed to astyle.",
Expand Down
4 changes: 2 additions & 2 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ class JupyterlabCodeFormatterClient {
});
}

public getAvailableFormatters() {
return this.request('formatters', 'GET', null);
public getAvailableFormatters(cache: boolean) {
return this.request('formatters' + (cache ? '?cached' : ''), 'GET', null);
}

public getVersion() {
Expand Down
16 changes: 12 additions & 4 deletions src/formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ class JupyterlabCodeFormatter {
code: string[],
formatter: string,
options: any,
notebook: boolean
notebook: boolean,
cache: boolean
) {
return this.client
.request(
'format',
'format' + (cache ? '?cached' : ''),
'POST',
JSON.stringify({
code,
Expand Down Expand Up @@ -158,7 +159,8 @@ export class JupyterlabNotebookCodeFormatter extends JupyterlabCodeFormatter {
currentTexts,
formatterToUse,
config[formatterToUse],
true
true,
config.cacheFormatters
);
for (let i = 0; i < selectedCells.length; ++i) {
const cell = selectedCells[i];
Expand Down Expand Up @@ -217,7 +219,13 @@ export class JupyterlabFileEditorCodeFormatter extends JupyterlabCodeFormatter {
this.working = true;
const editor = editorWidget.content.editor;
const code = editor.model.value.text;
this.formatCode([code], formatter, config[formatter], false)
this.formatCode(
[code],
formatter,
config[formatter],
false,
config.cacheFormatters
)
.then(data => {
if (data.code[0].error) {
void showErrorMessage(
Expand Down
53 changes: 28 additions & 25 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ class JupyterLabCodeFormatter
this.editorTracker
);

this.checkVersion().then(versionMatches => {
this.checkVersion().then(async versionMatches => {
if (versionMatches) {
this.setupSettings();
await this.setupSettings();
this.setupAllCommands();
this.setupContextMenu();
this.setupWidgetExtension();
Expand Down Expand Up @@ -125,18 +125,20 @@ class JupyterLabCodeFormatter
}

private setupAllCommands() {
this.client.getAvailableFormatters().then(data => {
const formatters = JSON.parse(data).formatters;
const menuGroup: Array<{ command: string }> = [];
Object.keys(formatters).forEach(formatter => {
if (formatters[formatter].enabled) {
const command = `${Constants.SHORT_PLUGIN_NAME}:${formatter}`;
this.setupCommand(formatter, formatters[formatter].label, command);
menuGroup.push({ command });
}
this.client
.getAvailableFormatters(this.config.cacheFormatters)
.then(data => {
const formatters = JSON.parse(data).formatters;
const menuGroup: Array<{ command: string }> = [];
Object.keys(formatters).forEach(formatter => {
if (formatters[formatter].enabled) {
const command = `${Constants.SHORT_PLUGIN_NAME}:${formatter}`;
this.setupCommand(formatter, formatters[formatter].label, command);
menuGroup.push({ command });
}
});
this.menu.editMenu.addGroup(menuGroup);
});
this.menu.editMenu.addGroup(menuGroup);
});

this.app.commands.addCommand(Constants.FORMAT_COMMAND, {
execute: async () => {
Expand All @@ -155,19 +157,20 @@ class JupyterLabCodeFormatter
});
}

private setupSettings() {
private async setupSettings() {
const self = this;
Promise.all([this.settingRegistry.load(Constants.SETTINGS_SECTION)])
.then(([settings]) => {
function onSettingsUpdated(jsettings: ISettingRegistry.ISettings) {
self.config = jsettings.composite;
}
settings.changed.connect(onSettingsUpdated);
onSettingsUpdated(settings);
})
.catch((error: Error) => {
void showErrorMessage('Jupyterlab Code Formatter Error', error);
});
try {
const settings = await this.settingRegistry.load(
Constants.SETTINGS_SECTION
);
function onSettingsUpdated(jsettings: ISettingRegistry.ISettings) {
self.config = jsettings.composite;
}
settings.changed.connect(onSettingsUpdated);
onSettingsUpdated(settings);
} catch (error) {
void showErrorMessage('Jupyterlab Code Formatter Error', error);
}
}

private setupCommand(name: string, label: string, command: string) {
Expand Down

0 comments on commit a8a063e

Please sign in to comment.