Type cleanup

[wch]

This is the second pull request described in #566. It adds many types and changes to the logic. The goal is to pass pyright strict mode type checks.

• Before merging, change main.yml back so that workflow is run only on pull requests that target master.

[wch]

For tests to pass, I added this "Content-Type": "application/json" header to some of the mock HTTP endpoints. I believe that's the correct behavior to mock the endpoint.

[github-actions]

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
4663	3508	75%	0%	🟢

New Files

No new covered files...

Modified Files

File	Coverage	Status
rsconnect/actions.py	48%	🟢
rsconnect/actions_content.py	65%	🟢
rsconnect/api.py	71%	🟢
rsconnect/bundle.py	80%	🟢
rsconnect/environment.py	84%	🟢
rsconnect/http_support.py	83%	🟢
rsconnect/json_web_token.py	97%	🟢
rsconnect/log.py	69%	🟢
rsconnect/main.py	62%	🟢
rsconnect/metadata.py	91%	🟢
rsconnect/models.py	92%	🟢
rsconnect/utils_package.py	77%	🟢
TOTAL	77%	🟢

updated for commit: 89f855d by action🐍

[wch]

This is ready for a review and then we can merge it into the type-cleanup branch.

There are still some typing issues that remain to be fixed, but the vast majority of the code now is fully typed. I think it's better to merge this sooner than later, because the typing updates will benefit other developers, and if we leave this branch open for a long time, there will be merge conflicts. After this is merged, then we can gradually fix the remaining issues.

Note that the pyright test in the Makefile is currently run with a leading -, so that even if it fails, make will not interpret it as an error, and CI will keep running. Once the typing work is completed, this can be removed so that any pyright failures will lead to CI failures.

[mmarchetti]

Can you clarify the purpose of the overloads vs. just having the one method with a boolean parameter?

[wch]

The purpose of the overloads is for type narrowing. Keep in mind that this function takes a response object and either throws or returns that same object.

• When is_httpresponse=False (the default), then the type checker can be certain that, the returned object is not a HTTPResponse. (If the object was an HTTPResponse, the function throws.) In all cases where this is used, the input type is either a TypedDict or a HTTPResponse, and so when it returns, we can be certain that the result is the TypedDict.
• When is_httpresponse=True, then the returned object could be an HTTPResponse. I added this because there are some instances where the input is expected to be an HTTPResponse. These are cases where the reponse status is 2xx, but the payload is not JSON; instead, it may be the contents of a file.

This stuff is a consequence of the _do_request method, which may return either an HTTPResponse or, if the reponse body is JSON, it will convert that to a dict and return that. The mix of possible return values makes some things downstream of that a bit awkward. I think that code would benefit from being restructured in a future PR.