feat: Add ifm support, bump version#3
Conversation
alxboyle
requested review from
jiatolentino,
kanewilliams and
kw-datamasque
and removed request for
kanewilliams and
kw-datamasque
| [project] | ||
| name = "datamasque-cli" | ||
| version = "1.0.0" | ||
| version = "1.1.0" |
There was a problem hiding this comment.
Reminder to rebase and bump to v1.2.0
| @@ -1,5 +1,42 @@ | |||
| # Changelog | |||
|
|
|||
| ## v1.1.0 | |||
| if is_json: | ||
| print_json(result.data) | ||
| else: | ||
| for record in result.data or []: | ||
| typer.echo(json.dumps(record, default=str)) |
There was a problem hiding this comment.
print_json(result.data) passes potentially-None data directly. The else NDJSON path guards with result.data or []. Change to print_json(result.data or [])?
| ) -> None: | ||
| """List all IFM ruleset plans.""" | ||
| client = get_ifm_client(profile) | ||
| plans = client.list_ruleset_plans() |
There was a problem hiding this comment.
Do we need a try/except DataMasqueApiError here? Same for get_ruleset_plan (line 101) and verify_token (line 261).
alxboyle
force-pushed
the
add-ifm-support
branch
4 times, most recently
from
8455c45 to
fddb26d
Compare
|
|
||
| ## v1.2.0 | ||
|
|
||
| ## Added |
There was a problem hiding this comment.
Nice catch. Fixed.
| if isinstance(error, str): | ||
| return error | ||
| if "detail" in body: | ||
| return str(body["detail"]) |
There was a problem hiding this comment.
Please confirm if the IFM server would produce {"detail": "string"}. It seems that key only appears in Pydantic 422 errors, whereas FastAPI might set it to e.errors() (a list of dicts).
There was a problem hiding this comment.
Good catch. Fixed.
There was a problem hiding this comment.
The NDJSON output path (--no-json) has no test. Could you add a test for it as well?
3 participants
No description provided.