Add type annotations #15
Conversation
|
WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue. |
|
I was just thinking about doing this. |
|
It's incompatible with python2. Is that a problem? Well, it could be done with py2, but it's a mess. |
| if isinstance(obj, datetime.datetime): | ||
| return obj.isoformat() | ||
| else: | ||
| super(json.JSONEncoder).default(obj) | ||
| return super().default(obj) |
There was a problem hiding this comment.
Nice that you were able to catch this using typing hints.
That's a good question. I don't think it is but let me confirm. |
mdellweg
force-pushed
the
type_annotations
branch
3 times, most recently
from
4c09dd1
to
32a23a5
Compare
|
now with |
FWIW We don't support py2 anyway to this point: pulp-cli/.github/workflows/main.yml Line 23 in c7ca274 |
mdellweg
force-pushed
the
type_annotations
branch
3 times, most recently
from
2bf59ba
to
fc7f608
Compare
mdellweg
force-pushed
the
type_annotations
branch
4 times, most recently
from
28092af
to
40c2789
Compare
| @@ -15,7 +15,7 @@ options include: | |||
| ## Configuration | |||
|
|
|||
| The CLI can be configured by using a toml file. | |||
| By default the location of this file is `~/.config/pulp/settings.toml`. | |||
| By default the location of this file is `~/.config/pulp/settings.toml`. | |||
| @click.option("--chunk-size", default=1000000, help="Chunk size in bytes (default is 1 MB)") | ||
| @click.option( | ||
| "--chunk-size", default=1000000, type=int, help="Chunk size in bytes (default is 1 MB)" | ||
| ) |
There was a problem hiding this comment.
It is adding type=int and i hope that will add input validation on the click side.
Not sure if the decorator will add type annotations to the variable passed to the command, but that would be super cool.
| def status(ctx: click.Context) -> None: | ||
| pulp_ctx: PulpContext = ctx.find_object(PulpContext) | ||
| result = pulp_ctx.call("status_read") | ||
| pulp_ctx.output_result(result) |
There was a problem hiding this comment.
I'm curious why this changed as compared to say orphans_delete()?
There was a problem hiding this comment.
I didn't know about find_object but it looks like you can embed more than one object in the context that way.
I think, this is more explicit and forgot about orphan_delete it seems.
| click.echo(".", nl=False, err=True) | ||
| raise click.ClickException("Task timed out") | ||
|
|
||
|
|
||
| def _config_callback(ctx, param, value): | ||
| def _config_callback(ctx: click.Context, param: Any, value: str) -> None: |
There was a problem hiding this comment.
I think param could maybe be a str but I guess it doesn't make a huge difference since we're not using it.
There was a problem hiding this comment.
I think, str didn't work...
mdellweg
force-pushed
the
type_annotations
branch
from
40c2789
to
576a2bf
Compare
Started to add type annotations.
This already revealed small inconsistencies (mostly around str vs bytes) and one actual bug (the
supercall).