Migrate to pydantic #214
Merged
Migrate to pydantic #214
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Also makes get_list do the logic of checking hit limits and exception handling. - Only the API module has any idea of endpoint addresses for pydantic calls. The rest of the client code does not care. - Implements a IPAddressField for Hosts to reduce the number of str-fields we have. We should ideally have none.
* Clean up authentication mess. - Cleans up the names for the auth functions. - Adds a LoginFailedError exception. - Passes on params from the pre-relogin command to the post-login command. - Reintroduces the previous behaviour where failing the login upon startup exists mreg.
* Allow oneshot commands from the terminal. This patch allows the use of mreg-cli without interaction, ala: `mreg-cli host info foo` This will then feed the command `host info foo` to the cli and then exit. Also adds an option to force token only login. Good for scripts. :)
- No micromanagment of error types, just dump the error text back to the user.
## The problem...
It was much too easy to do `host remove foo -f` and have it bite in the worst possible way.
## The solution?
In this PR, `-force` is no longer a catch-all for making `host remove`, well, remove the host. If the host has cnames, ptrs, mx, srvs or ipadresses across multiple vlans, the user needs to declare a desire to override each of these explicitly via the new option `-override`.
## Supported input
Accepted overrides are `cname`, `ipadress`, `mx`, `srv`, `ptr`, `naptr`. Invalid overrides offered as parameters to `-override` are errors and the command will report on the unexpected input and stop before executing anything.
The choice to have these as textual inputs is intended with the explicit goal of making their use require more than tab-completing an option.
## Example usage
`host remove foo -force -override cname,ipaddress,mx`.
This would allow deletion / removal of a host with cnames, ipadresses across different vlans, and mx set. However, any ptr or srv RRs will still cause the deletion to cancel.
## Example warning from `host remove`
```
WARNING: : bar.example.org requires force and cnames as overrides for deletion:
1 cnames, override with 'cname'
- fubar.example.org
multiple ipaddresses on the same VLAN. Must use 'force'."
```
## Notes:
1. `-force` alone works on multiple ipadresses from the same VLAN.
2. `-override` requires the presence of `-force`. The documentation states this both from expanded inline help and `host remove -h`.
---------
Co-authored-by: pederhan <pederhan@usit.uio.no>
|
I've used Pydantic for a few CLI projects by now, and there are a couple of points I think you should have in the back of your mind when developing this further:
Regarding point 2, it's important to address this IMO, as the FWIW, I don't have good answers to these questions. You want to balance reliability and ease of use with correctness and debuggability, and those factors can be at odds with each other at times. |
|
Yeah, I've been down the same thoughts myself. Tbh, these kinds of problems are why I'm looking at gRPC for other projects, and also why they tend to come in Rust. Which doesn't really help the here and now. :p |
- Also differentiates between get_list and get_list_unique where one expects many or one hit. - Further improve models with better cleaning of macaddress. - Avoid string literals for endpoints, use an Enum with optional identifier application.
- api.get_host now allows for lookups by id, ipaddress, macaddress or hostname, and if all that fails checks for CNAMEs. - Add network endpoints and more. - Make models frozen so they can be used as hash keys, returning new objects of one tries to assign to them via acceptable methods. __setattr__ and __delattr__ are overridden to cause AttributeErrors if used. - Migrate `host add` and `host remove` - Document (some) of the older API calls with exceptions.
- Host output now uses class methods from the models involved to print multiple entries. - clean_hostname is copied around a bit to support old and new code... :( - Models with a host parameter also get a method to resolve the host. - Added support for created_at and updated_at.
- We don't use clean_hostname, we have a hostname type. - Make roles look sane for the consumer of the cli.
- Models know their endpoint - This allows for class.get, class.create, obj.delete, obj.refetch, obj.patch - patch/create return the new obj - Needs refactoring of model file, it's getting large.
…, the models don't know and don't care.
- This does not rely on getting the IP from the server first, it uses the servers API directly.
pederhan
reviewed
Apr 23, 2024
* Validate responses with Pydantic * Specify part of statement `type: ignore` applies to * Replace all `get_list` calls with `get_typed` * Use `TypeAliasType` from typing-extensions * Revert accidental find and replace change * HACK: Use `cast()` in `get_list_unique()` * Add source url comment to recursive JSON type * Add helper functions for response validation * Remove `ResponseLike` * Raise MREG ValidationError on failed validation * Validate response in `get_list_unique` * Improve exception handling for failed validation * Add `HistoryResource` from #232 Also directly validates results to `HistoryItem` instead of going through a dict.
* Strip None values when sending JSON * Recursively strip
* Require SOCKS dependencies * Add pyinstaller dev dependency
- This PR allows for copying of all policies (roles) from one host to another via a new command, `policy host_copy`.
* Support multiple destinations in host_copy. - Also reduces output noise. - Also handles the situation where a destination already has the role in question. - Requires Role to be hashable.
* Publishing * Add branch condition to publish workflow * Wording * Remove usage of requirements.txt files * Add basepython resolution for numbered envs * Use uv in tox workflows * Unfinished publish workflow * trying stuff * Do not specify tox env in build workflow * Move pyinstaller dependency to tox * Actually run tox * Fix binary dist path * Revert incorrect binary path * Fix underscore in artifact name * verbose pypi * Use test pypi * Use `mreg-cli-v*` tag pattern * Dump workflow info * Add branch check * Try to create release * Try to use trusted publishing * Remove unused pypi vars * Try to rename binaries * Try to actually read actions docs * Fix bin name * Publish a4 * Rename binary after creation * Remove publishing branch * Make built binary executable * Remove executable step * Build on master and publish to real pypi * Update publishing instructions * Exit explicitly only on master branch * Add artifact and version info, move `-desc`to Removed * Final publishing test run * Enable master protection again * Make GitHub release non-prelease * Add missing newline * Fix CONTRIBUTING markdown lint violations Does not "fix" the ordered list rule violation, since indenting the code blocks looks very awkward.
* Add authors to pyproject.toml * Add historical authors * Add AUTHORS file
* Make the cli exceptions write to outputmanager * Replace formatted datetime strings in output * Fix workflow annotations about GH action versions * Update testsuite json with modified label commands * Workaround for json-incapable server * Also use workaround for patch requests * Use OutputManager instead of the cli_* methods * Use specialized exceptions. (#256) - This PR migrates all use of CliError and CliWarning to more specialized exceptions. - Also fix a bug with handling broken filter expressions. - Prefixes Errors with ERROR as not everyone has color vision. * Cull utility functions that aren't used anymore * Further culling of... old stuff. * Fix valid_numeric_ttl min and max value handling * Use `APIMixin.patch()` in `Zone.set_default_ttl()` * Add missing OK message for `host add` * Remove IP/network from `host add` OK message * Verified behaviour for all zone commands I have looked at the new output and api calls for the "zone" commands in the testsuite and it looks good to me. Updating testsuite-result.json to reflect that. * Verified behaviour for all group commands I have looked at the new output and api calls for the "group" commands in the testsuite and it looks good to me. Fixed a bug in history.py concerning how history is printed when owners or hosts are added/removed from hostgroups. Made a minor change to the datetime regex in diff.py to handle the extra space that comes before a single digit date. * Fix fetching network from location header (#258) * Extract fetching resource by location to separate method Fixes failing `network create` tests * Use `field_for_endpoint()` * Prefer using location header verbatim Adds handling for endpoints that return invalid location headers. * Remove `field_for_endpoint()` * Remove placeholder comment, move src comments * Remove special label handling, never re-fetch labels * Add a check for frozen/force to "host add" We forgot to check if the network was frozen in "host add". Added a check for that in this commit, but we'll probably have to do something similar in a_aaaa.py. Went through more command output and verified it. Mostly network commands this time. Modified diff.py to not care about length of white space. Since ip addresses can vary between runs, formatted output can vary in length, spesifically how much whitespace is added. * Fix fetching paginated endpoints twice in `get_list_generic` (#261) * Fix fetching paginated endpoints twice in `get_list_generic` * Break instead of return * Refactor `get_list_generic` result checking * Fix network excluded range output. - Also use IPAddressField for ips in excluded ranges. * Fix delegeation response. - Add support for "delegation" scope akin to "zone" scope in server responses for zones. * Implement logging. (#266) * Implement logging. - Migrates out logging and recording commands to their own command files. - During the logging setup there was an issue trying to untangle commands and conventions, so most commands are now refactored into proper command files. - Add logging to API calls and output. - Move CliExit to exceptions.py... - When logging API calls, also logs the clients corrolation ID (for requests and respones) and the servers request ID (per request/response) for easier server side debugging. :) - Restrict logging (to errors only) when running the test suite, it's... noisy. :D --------- Co-authored-by: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com> * Support sending more than str to post. (#264) * Support sending more than str to post. Also fix some ruffs. * Consistent signatures for get_list_generic. --------- Co-authored-by: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com> * Fix a bug in remove_excluded_range models.Network.remove_excluded_range had a bug where it wouldn't match the supplied parameters even when a range existed. Solved by converting the ip address values to strings. This commit also updates the rest of the "network" part of the test suite results. * Add network set_category/set_location commands * Escape error messages (#269) * Escape exception messages in HTML text * Only escape message, not formatted exc * Escape messages only in formatted exceptions (#271) * Escape messages only in formatted exceptions * Rename `escaped()` to `escape()` * Add `QueryParams` type (#273) * Add `QueryParams` type * Remove redundant dict comprehension * Add interactive diffing (#270) Revert "Use dumb terminal only for diff.py" This reverts commit 42a70de. Use dumb terminal only for diff.py Remove vertical borders for easier copying Add comment about review mode Invert conditional Docstrings, ordering Comments, names, docstrings Don't review in CI Change choices (order + choice) Remove unused state Revert "Run diff.py on testsuite results" This reverts commit def4bc5. Run diff.py on testsuite results Refactor diff.py Add interactive diffing * Add force req for frozen nets to a_add, aaaa_add Also: - Add "rich" to requirements-dev.txt, it was missing - Update testsuite-result.json with more verified test output * Fix problems with history output Also, update more test resuls * Add minor quality of life + message changes to diff.py (#275) * Remove `getattr()` usage in `_request_wrapper()`. (#274) * Remove LoginFailedError message hard coding. (#277) * Remove hardcoded LoginFailed output. Fixes #272 * Fix quote usage and handle direction (im)possibly being unassigned. (#279) * Add a regex for macaddress to diff.py Macaddresses were caught by the IPv6 regex, which made certain commands in the testsuite look like the wrong type of parameter was supplied. * Add OK message to a_add when success Also, avoid refetching the host twice. * Remove unnecessary warning from dhcp assoc dhcp assoc <hostname> <ip> would warn that hostname isn't a valid ip address. This commit changes the code so the exception isn't raised but otherwise it works the same. Also: - A minor change to an error message about a host having multiple ips. - Verified the output from the rest of the commands in the dhcp section. * Remove stripping of None values from PATCH calls (#282) Also ensures nullable and non-nullable TTL fields are treated differently. * Improve diff numbering + messages (#284) * Add number to diff when reviewing * Improve diff messages * Handle `DiffError` * Add `ERROR` msg prefix for exceptions * Fix inconsistent stdout/stderr printing * Remove parenthesis around plural s * Fix `host set_comment` with empty string (#283) * Use Pydantic v2 style model config in `FrozenModel` (#285) * Use object repr when PATCH validation fails * Improve type safety of validators (#286) * Improve type safety of validators * Remove unused import * Improve safety of TTL methods for non-nullable fields (#287) * Improve safety of TTL methods for non-nullable fields * Add external id field for `Endpoint.Nameservers` * Add QueryParam annotations for inline params * Replace IDs & IPs in test result URLs (#288) * Replace IDs & IPs in test result URLs * Simplify `unquote_url` * Expand match group explanations * Fix comment typo * A few code changes + changes to testsuite-result (#289) - Changes to testsuite-result.json that came from #288 - The rest of the changes to testsuite-result.json, manually verified - a_aaaa.py: Add a message and a requirement for force when adding an additional IP address to a host. - rr.py: Replace occurrences of str(host.id) with host.id in API requests. * Write back original lines in diff review (#290) * Write back non-placeholder lines in diff reviews * Refactor results to classes * Rename `TestSuiteLog` to `TestSuiteResult` * Delete .pyi file * Fix missing None check * Accept int values in queries * Add missing annotations import * Use uv in CI (#292) * Use uv in CI * Use official uv action * Add build dependencies to pyproject.toml * Use uv run? * Set up python with uv in CI * idk * Use venv * uv run? * Use uv python when building too * Remove system python env var --------- Co-authored-by: pederhan <pederhan@usit.uio.no> Co-authored-by: Terje Kvernes <terjekv@users.noreply.github.com> Co-authored-by: Terje Kvernes <terje@kvernes.no> Co-authored-by: Peder Hovdan Andresen <107681714+pederhan@users.noreply.github.com>
* Build PyInstaller binary in CentOS 8 container * Use single release step
* Fix cli.py type checking errors * Fix comment * Explain noqa Ruff codes
* Add error msg builder with underlined suggestions * Refactor error message building * Add docstrings
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another large scale rewrite to move away from a bundle of dicts into something actually structured into Pydantic models. The goal here is to be able to add features and output data without needing to test, guess, and generally fiddle with dicts everywhere...