Replace type hints for Sequence CORSMiddleware class init parameters to Iterable.

[stankudrow]

Summary

Provide more suitable type hints for CORSMiddleware class on the trail of the discussion #2722 .

@roy-work proposed typing.Container[str] for allow_ parameters in the CORSMiddleware class constructor. I guess that typing.Iterable[str] may be more suitable because of some preprocessing within the __init__() method, so mere Containers cannot be used and mypy will complain about it.

Also the conversion to set data structure for changed parameters is added due to possibly given generators, which are iterable as well and can be exhausted. Moreover, sets are nice against possible duplicates.

And, as the final touch, I gather that adding --maxfail=1 option for pytest seems nice: no need in waiting for all tests to be executed, but I can remove this option if the community explain me the reasons (please).

UPD: docs are updated including markdown linting complaints addressed.

Checklist

• I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
• I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
• I've updated the documentation accordingly.

[Kludex]

UPD: docs are updated including markdown linting complaints addressed.

Please revert those changes.

[Kludex]

And, as the final touch, I gather that adding --maxfail=1 option for pytest seems nice: no need in waiting for all tests to be executed, but I can remove this option if the community explain me the reasons (please).

Please revert. People can add that option if they prefer when running ./scripts/test --max-fail=1.

[Kludex]

Didn't you want to add typing.Container?

[stankudrow]

UPD: docs are updated including markdown linting complaints addressed.

Please revert those changes.

Hello, do you mean all of them or some? In the second case, which ones should be reverted?

[stankudrow]

Didn't you want to add typing.Container?

I tried it first time and mypy emitted complains, e.g. starlette/middleware/cors.py:60: error: Argument 1 to "join" of "str" has incompatible type "Container[str]"; expected "Iterable[str]" [arg-type].

I presume that we are talking about incoming parameters, which are "perfectly" fine to be iterable objects because we can iterate over them and gather their items. Moreover, these type hints cover generators (which are iterators, which are iterable objects) as well which explains the rationale over converting the arguments into sets with collateral deduplication. When these incliming allow_ and expose_ parameters get assigned to a class instance, these attributes (already preprocessed with necessity of iterability property) are used as containers.

Conclusion: just because we use some attributes as containers so far, it should not restrict corresponding attributes to the same type.

[stankudrow]

And, as the final touch, I gather that adding --maxfail=1 option for pytest seems nice: no need in waiting for all tests to be executed, but I can remove this option if the community explain me the reasons (please).

Please revert. People can add that option if they prefer when running ./scripts/test --max-fail=1.

The option --maxfail=1 was removed.

[Kludex]

@stankudrow There are too many changes here. The discussion that motivated this PR just mentions that we can change a type hint to become typing.Container. If that's not possible, then I'm fine with not changing anything.

[jonathanberthias]

I tried it first time and mypy emitted complains, e.g. starlette/middleware/cors.py:60: error: Argument 1 to "join" of "str" has incompatible type "Container[str]"; expected "Iterable[str]" [arg-type].

Instead of Container, you can try with Collection as it is both a Container and an Iterable. Then you shouldn't have to change the code much I think, but it would allow passing a set as argument.

[stankudrow]

I tried it first time and mypy emitted complains, e.g. starlette/middleware/cors.py:60: error: Argument 1 to "join" of "str" has incompatible type "Container[str]"; expected "Iterable[str]" [arg-type].

Instead of Container, you can try with Collection as it is both a Container and an Iterable. Then you shouldn't have to change the code much I think, but it would allow passing a set as argument.

https://docs.python.org/3/library/collections.abc.html#collections.abc.Collection - I am not sure the "sized" property is something to impose because iterability is quite enough for current and, I guess, future preprocessing which has not changed so much. Also, I am perfectly fine with "setting" incoming parameters for deduplication sake + a protection for possible exhaustive objects, like generators, that are iterable and, therefore, valid by their type as well.

[stankudrow]

@stankudrow There are too many changes here. The discussion that motivated this PR just mentions that we can change a type hint to become typing.Container. If that's not possible, then I'm fine with not changing anything.

Not as much as it appears: just one module and all about mostly refactoring with collateral deduplication. I see the work done, but I still don't understand what exactly should be reverted concerning the docs/middleware.md file.

IMHO, I didn't see any chance for a mere type hint switch towards Container[str]. I came to the Iterable[str] case and I understand that generator objects can be passed legitimately, so turning them into sets seems reasonable even if it make code a little "defensive".

[stankudrow]

@Kludex , rebased.

[Kludex]

@stankudrow There are too many unrelated changes here.