GraceDB Client merge requestshttps://git.ligo.org/computing/gracedb/client/-/merge_requests2024-03-10T21:36:07Zhttps://git.ligo.org/computing/gracedb/client/-/merge_requests/89catch decoding error (barf) and return message to user2024-03-10T21:36:07ZAlexander Pacecatch decoding error (barf) and return message to userhttps://git.ligo.org/computing/helpdesk/-/issues/5596https://git.ligo.org/computing/helpdesk/-/issues/5596https://git.ligo.org/computing/gracedb/client/-/merge_requests/87Fixes to docs and introduction of doctest2023-12-04T23:25:22ZDaniel WysockiFixes to docs and introduction of doctest- Added a new unit test that runs all code examples using `doctest`
- This required mocking up `GraceDb` (to avoid making actual requests, except for fetching some initial data, which it restricts to `gracedb-test`) and `builtins.open`...- Added a new unit test that runs all code examples using `doctest`
- This required mocking up `GraceDb` (to avoid making actual requests, except for fetching some initial data, which it restricts to `gracedb-test`) and `builtins.open` (to pretend files like `/path/to/fake/file.xml` were real)
- Fixed many of the code examples which were either stuck on Python 2 syntax or just wrong
- Removed `from io import open` in `rest.py`
- In Python 2 the built-in `open` was different from `io.open`. Now they are aliases.
Some important caveats:
- A number of examples are not actually run, using the `doctest:+SKIP` directive through a trailing in-line comment. This is a potential area for future improvement, but probably fine as-is. Things currently being skipped:
- Expected HTTP status codes and response bodies did not match the example, given that they were mocked up to do nothing.
- The `GraceDb.files()` method would have required a bit more mocking, so I skipped instead
- Any markup files (e.g., our Sphinx docs) are not run with doctest. This can be added in the future, but would require
1. Adding the [`doctest` Sphinx extension](https://www.sphinx-doc.org/en/master/usage/extensions/doctest.html)
2. Adding the `.. doctest::` directive before each example
3. Figuring out some way to run these tests with our mocked `GraceDb` class and `builtins.open` (probably possible through Sphinx's `conf.py`
- I did still read over all of the code examples in markup files and confirmed there are no visible errors, including function signature mistakes, though I did not actually run these examples.
- Each `doctest` example is potentially creating a new `GraceDb` session, if it calls `GraceDb`. We're already doing this in the other unit tests, so it's probably not a huge amount of extra overhead, but there are quite a few of these so we should be careful that we're not negatively impacting service performance.
- We still use the `GraceDb.camelCase` versions of methods, which are backwards compatible aliases to our newer, preferred `GraceDb.snake_case` versions. We probably want to change all of these to avoid encouraging use of old names. We could even cause the unit test to fail if an alias is used instead of the original name.Daniel WysockiDaniel Wysocki