Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve error messages for absent/invalid schema path #22

Merged
merged 9 commits into from
Sep 1, 2022

Conversation

dlqqq
Copy link
Contributor

@dlqqq dlqqq commented Aug 31, 2022

The error messages for passing a path without a file present or for passing a string path (rather than a Pathlib object) to EventLogger#register_event_schema() were very obtuse. This PR addresses that and improves error output for both cases. Also:

  • updates documentation to specify that users must pass a Pathlib object to register_event_schema().
  • improve error catching and handling _load_schema(). adds new exceptions
    • raises exception when user loads un-deserializable (is there a better adjective for this?) schema file via passing a PurePath. This was previously uncaught by EventLogger, and was caught by JSON validator farther downstream instead.
  • sets pre-commit node version to system
  • adds docs link to README

Before

Passing a path without a file:

dev-dsk-dlq-2b-55651109 % jupyter events validate asdf.txt
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Validating the following schema ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Traceback (most recent call last):
  File "/workplace/dlq/jupyter_events/jupyter_events/cli.py", line 39, in validate
    _schema = EventSchema._load_schema(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 66, in _load_schema
    raise EventSchemaLoadingError(
jupyter_events.schema.EventSchemaLoadingError: When deserializing `schema`, we expected a dictionary to be returned but a <class 'str'> was returned instead. Double check the `schema` to make sure it is in the proper form.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/dlq/anaconda3/envs/jserver-dev/bin/jupyter-events", line 8, in <module>
    sys.exit(main())
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/workplace/dlq/jupyter_events/jupyter_events/cli.py", line 42, in validate
    _schema = EventSchema._load_schema(schema_path)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 74, in _load_schema
    _schema = yaml.load(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/yaml.py", line 23, in load
    data = pathlib.Path(str(fpath)).read_text()
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/pathlib.py", line 1266, in read_text
    with self.open(mode='r', encoding=encoding, errors=errors) as f:
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/pathlib.py", line 1252, in open
    return io.open(self, mode, buffering, encoding, errors, newline,
  File "/home/dlq/anaconda3/envs/jserver-dev/lib/python3.9/pathlib.py", line 1120, in _opener
    return self._accessor.open(self, flags, mode)
FileNotFoundError: [Errno 2] No such file or directory: 'asdf.txt'

Passing a string path to EventLogger#register_event_schema():

>>> logger = EventLogger()
>>> logger.register_event_schema("test_schema.json")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workplace/dlq/jupyter_events/jupyter_events/logger.py", line 134, in register_event_schema
    event_schema = self.schemas.register(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema_registry.py", line 42, in register
    schema = EventSchema(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 44, in __init__
    _schema = self._load_schema(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 66, in _load_schema
    raise EventSchemaLoadingError(
jupyter_events.schema.EventSchemaLoadingError: When deserializing `schema`, we expected a dictionary to be returned but a <class 'str'> was returned instead. Double check the `schema` to make sure it is in the proper form.

After

Passing a path without a file:

dev-dsk-dlq-2b-55651109 % jupyter events validate asdf.txt
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Validating the following schema ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
[20:30:04] ERROR: File not present at path "asdf.txt".                                                                         

Passing a string path to EventLogger#register_event_schema():

>>> from jupyter_events import EventLogger
>>> logger = EventLogger()
>>> logger.register_event_schema("test_schema.json")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workplace/dlq/jupyter_events/jupyter_events/logger.py", line 134, in register_event_schema
    event_schema = self.schemas.register(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema_registry.py", line 42, in register
    schema = EventSchema(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 49, in __init__
    _schema = self._load_schema(schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 105, in _load_schema
    EventSchema._ensure_yaml_loaded(loaded_schema)
  File "/workplace/dlq/jupyter_events/jupyter_events/schema.py", line 74, in _ensure_yaml_loaded
    raise EventSchemaLoadingError(error_msg)
jupyter_events.schema.EventSchemaLoadingError: Could not deserialize schema into a dictionary. Paths to schema files must be explicitly wrapped in a Pathlib object.

Notes

For some reason, pre-commit keeps failing my code while also not making any changes. Not sure if this will block the pipeline, so leaving a note here just in case.

(22-08-31 19:29:18) <1> [/workplace/dlq/jupyter_events]
dev-dsk-dlq-2b-55651109 % pre-commit
fix end of files.........................................................Passed
check for case conflicts.................................................Passed
check that executables have shebangs.................(no files to check)Skipped
fix requirements.txt.................................(no files to check)Skipped
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check toml...........................................(no files to check)Skipped
check yaml...............................................................Passed
debug statements (python)................................................Passed
forbid new submodules................................(no files to check)Skipped
check builtin type constructor use.......................................Passed
trim trailing whitespace.................................................Passed
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted jupyter_events/cli.py
reformatted tests/test_schema.py

All done! ✨ 🍰 ✨
2 files reformatted, 2 files left unchanged.

isort....................................................................Failed
- hook id: isort
- files were modified by this hook

Fixing /workplace/dlq/jupyter_events/jupyter_events/cli.py
Fixing /workplace/dlq/jupyter_events/tests/test_schema.py

mypy.....................................................................Passed
prettier.................................................................Passed
pyupgrade................................................................Passed

@dlqqq
Copy link
Contributor Author

dlqqq commented Aug 31, 2022

Precommit is still failing. Would like some help here, since this error is pretty opaque and I have no idea what's causing it.

@Zsailer Zsailer added the enhancement New feature or request label Aug 31, 2022
README.md Outdated Show resolved Hide resolved
jupyter_events/cli.py Outdated Show resolved Hide resolved
jupyter_events/schema.py Outdated Show resolved Hide resolved
@Zsailer
Copy link
Member

Zsailer commented Aug 31, 2022

You'll likely need to run pre-commit on these files locally. The "Contributing" docs in Jupyter Server describe this better: https://github.com/jupyter-server/jupyter_server/blob/main/CONTRIBUTING.rst#code-styling

We should probably add similar docs here.

@dlqqq
Copy link
Contributor Author

dlqqq commented Aug 31, 2022

@Zsailer I was running it locally, but for some reason black would try to correct changes by isort, which would then try to correct changes by black, meaning that precommit would always fail. Looks like you need to manually specify black compatibility to isort. Check this out:

PyCQA/isort#1518

@blink1073 You know any other repos where this change should be made?

@Zsailer Zsailer merged commit 5f94151 into jupyter:main Sep 1, 2022
@Zsailer
Copy link
Member

Zsailer commented Sep 1, 2022

Thanks, @dlqqq!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants