fix(typing): use proper type for SinkCallable **kwargs

[jap]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b541976) to head (c4af605).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #2411   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           64        64           
  Lines         7726      7726           
  Branches      1071      1071           
=========================================
  Hits          7726      7726           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[CaselIT]

Hi,

The proposed typing is wrong. kwargs are not typed as dict.

Also when can you have none as the value?

[jap]

Oh right, that kwargs thing not being a dict is one of those typing gotchas.

Anyway, to answer your question, kwargs is constructed using groupdict() which can return items with None when a regex matches but individual named groups don't, for example:

>>> re.match('(?P<foo>bar)?', 'baz').groupdict()
{'foo': None}

I'll give it another thought - what I'd like is being able to have my functions implement SinkCallable with a call signature

def my_sink(req: falcon.Request, resp, falcon.Response) -> None:
  ...

but mypy and other type checkers cannot infer that **kwargs is going to be empty in those cases so there will be a type mismatch.

I guess for now I'll have to write it as

def my_sink(req, resp, **_kwargs: object) -> None:
  assert not _kwargs, 'got unexpected extra arguments in my_sink'
  ...

which somewhat nullifies the elegance of passing these things as keyword arguments :(

Long term I can imagine having add_sink require an explicit string or regex as prefix (and no more implicit coercion to regex), so that it would be possible to have a typing-overload for add_sink that accepts a SinkCallable without kwargs if a string is passed as prefix. However, this would mean a breaking change for people passing regular expressions as strings to add_sink.

[CaselIT]

kwargs is constructed using groupdict() which can return items with None when a regex matches but individual named groups don't, for example:

understood!

regarding the call signature for sink, maybe we can use @overload so that both types are accepted?

[vytas7]

Thanks for this improvement @jap, yes, the values may be None in the case of optional unmatched regex groups.

[CaselIT]

oh I wanted to try overload in this branch, but I guess we can do a follow on change for it

[vytas7]

Aha I thought you were planning a follow-up anyway since you approved 😅