Summary

Add framework for typing query arguments to methods.

Review Request #14339 — Created Feb. 10, 2025 and submitted March 20, 2025, 8:38 a.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

This change adds some machinery for rewriting query argument names so we
can more easily deal with API endpoints that use hyphens in parameters.

A couple new classes have been added for basic parameter lists for GET
operations on item and list resources, that include the built-in request
parameters that are supported for all resources.

I've also added additional TypedDicts for existing resource subclasses
which have special parameters for their GET operations. This makes it so
something like root.get_review_requests(branch=False) will pop up a
warning that the branch parameter must be str.

Testing Done


Ran unit tests.
Used a test script to verify that typing was correctly passed through

  for methods that take kwargs for query parameters.

Commits

Summary	ID
Add framework for typing query arguments to methods. This change adds some machinery for rewriting query argument names so we can more easily deal with API endpoints that use hyphens in parameters. A couple new classes have been added for basic parameter lists for GET operations on item and list resources, that include the built-in request parameters that are supported for all resources. I've also added additional TypedDicts for existing resource subclasses which have special parameters for their GET operations. This makes it so something like `root.get_review_requests(branch=False)` will pop up a warning that the `branch` parameter must be `str`. Testing Done: - Ran unit tests. - Used a test script to verify that typing was correctly passed through for methods that take kwargs for query parameters.	ccae3d5a8994a0556186a71f38655124406c67ba

Summary

Add framework for typing query arguments to methods.

This change adds some machinery for rewriting query argument names so we can more easily deal with API endpoints that use hyphens in parameters. A couple new classes have been added for basic parameter lists for GET operations on item and list resources, that include the built-in request parameters that are supported for all resources. I've also added additional TypedDicts for existing resource subclasses which have special parameters for their GET operations. This makes it so something like `root.get_review_requests(branch=False)` will pop up a warning that the `branch` parameter must be `str`. Testing Done: - Ran unit tests. - Used a test script to verify that typing was correctly passed through for methods that take kwargs for query parameters.

ccae3d5a8994a0556186a71f38655124406c67ba

Issues

Description	From	Last Updated
redefinition of unused 'Mapping' from line 13 Column: 5 Error code: F811	reviewbot	Feb. 10, 2025, 8:58 a.m.
Missing Version Added.	chipx86	March 16, 2025, 12:53 p.m.
Missing Version Added.	chipx86	March 16, 2025, 12:53 p.m.
These should be switched to keep sort order.	chipx86	March 16, 2025, 12:53 p.m.
Missing Version Added.	chipx86	March 16, 2025, 12:53 p.m.
We should use a Mapping here, so it's not typed as mutable.	chipx86	March 16, 2025, 12:54 p.m.
Should we take headers as Mapping?	chipx86	March 16, 2025, 12:56 p.m.
Missing Version Added.	chipx86	March 16, 2025, 12:56 p.m.
Is this because we transform query_args above? Rather than a cast, it might be more type-safe, if we define a …	chipx86	March 16, 2025, 12:56 p.m.
If we set the parent class to a Mapping, this will need to be changed too.	chipx86	March 16, 2025, 12:57 p.m.
Same with Mapping. Possibly others as well. I won't swamp the review with each instance.	chipx86	March 16, 2025, 12:58 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/api/resource/base.py (Diff revision 1)
The issue has been dropped. Show all issues
```
redefinition of unused 'Mapping' from line 13

Column: 5
Error code: F811
```

Commits:

	Summary	ID
	Add framework for typing query arguments to methods. This change adds some machinery for rewriting query argument names so we can more easily deal with API endpoints that use hyphens in parameters. A couple new classes have been added for basic parameter lists for GET operations on item and list resources, that include the built-in request parameters that are supported for all resources. I've also added additional TypedDicts for existing resource subclasses which have special parameters for their GET operations. This makes it so something like `root.get_review_requests(branch=False)` will pop up a warning that the `branch` parameter must be `str`. Testing Done: - Ran unit tests. - Used a test script to verify that typing was correctly passed through for methods that take kwargs for query parameters.	435efa40d5fdfa4079680362a0ab50bbabe04c07
	Add framework for typing query arguments to methods. This change adds some machinery for rewriting query argument names so we can more easily deal with API endpoints that use hyphens in parameters. A couple new classes have been added for basic parameter lists for GET operations on item and list resources, that include the built-in request parameters that are supported for all resources. I've also added additional TypedDicts for existing resource subclasses which have special parameters for their GET operations. This makes it so something like `root.get_review_requests(branch=False)` will pop up a warning that the `branch` parameter must be `str`. Testing Done: - Ran unit tests. - Used a test script to verify that typing was correctly passed through for methods that take kwargs for query parameters.	d13021e064b3427f1e9cd74d84f6cf9329f2278f

Diff:

Revision 2 (+882 -202)

Show changes

	rbtools/api/resource/base.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_file_attachment.py
	rbtools/api/resource/draft_diff_commit.py
	rbtools/api/resource/file_diff.py
	rbtools/api/resource/mixins.py
	rbtools/api/resource/review_request.py
	rbtools/api/resource/root.py
	1 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing Version Added.
```
rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing Version Added.
```

rbtools/api/resource/base.py (Diff revision 2)

The issue has been resolved. Show all issues

These should be switched to keep sort order.

rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing Version Added.
```
rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
We should use a Mapping here, so it's not typed as mutable.
```
rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Should we take headers as Mapping?
```
rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Missing Version Added.
```

rbtools/api/resource/base.py (Diff revision 2)

The issue has been resolved. Show all issues

Is this because we transform query_args above? Rather than a cast, it might be more type-safe, if we define a new variable of the correct type for the result, type it as needed, and set a default if query_args is empty. That'd be a bit more explicit.

rbtools/api/resource/base.py (Diff revision 2)
The issue has been resolved. Show all issues
```
If we set the parent class to a Mapping, this will need to be changed too.
```
rbtools/api/resource/diff_file_attachment.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Same with Mapping. Possibly others as well. I won't swamp the review with each instance.
```

Commits:

	Summary	ID
	Add framework for typing query arguments to methods. This change adds some machinery for rewriting query argument names so we can more easily deal with API endpoints that use hyphens in parameters. A couple new classes have been added for basic parameter lists for GET operations on item and list resources, that include the built-in request parameters that are supported for all resources. I've also added additional TypedDicts for existing resource subclasses which have special parameters for their GET operations. This makes it so something like `root.get_review_requests(branch=False)` will pop up a warning that the `branch` parameter must be `str`. Testing Done: - Ran unit tests. - Used a test script to verify that typing was correctly passed through for methods that take kwargs for query parameters.	d13021e064b3427f1e9cd74d84f6cf9329f2278f
	Add framework for typing query arguments to methods. This change adds some machinery for rewriting query argument names so we can more easily deal with API endpoints that use hyphens in parameters. A couple new classes have been added for basic parameter lists for GET operations on item and list resources, that include the built-in request parameters that are supported for all resources. I've also added additional TypedDicts for existing resource subclasses which have special parameters for their GET operations. This makes it so something like `root.get_review_requests(branch=False)` will pop up a warning that the `branch` parameter must be `str`. Testing Done: - Ran unit tests. - Used a test script to verify that typing was correctly passed through for methods that take kwargs for query parameters.	ccae3d5a8994a0556186a71f38655124406c67ba

Diff:

Revision 3 (+918 -248)

Show changes

	rbtools/api/request.py
	rbtools/api/resource/base.py
	rbtools/api/resource/diff.py
	rbtools/api/resource/diff_file_attachment.py
	rbtools/api/resource/draft_diff_commit.py
	rbtools/api/resource/file_diff.py
	rbtools/api/resource/mixins.py
	rbtools/api/resource/review_request.py
	2 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (f51423a)