https://authzed.com logo
Join Discord
Powered by
Add type hint support · Issue #140 · aut...
# spicedb
a
I filed a bug this week about type hints for the python library: https://github.com/authzed/authzed-py/issues/140 Are outside contributions welcome to help address the issue? I'm happy to spend some time on this myself.
v
yeah, totally welcome! 🙏
a
Do you have any opinions on creating an explicit AsyncClient class?
v
I think it's a great idea!
j
yeah, we tried doing it automagically
and I think that was a mistake
likely better to make it explicit
a
Finally got some free time: https://github.com/authzed/authzed-py/pull/149 Would a maintainer mind running the CI jobs to make sure the new one passes? Also I'm assuming you don't want to deprecate the existing client so I left it as-is
j
started CI
a
Made some fixes to pass the linter - can you run them again? P.s. the test.yaml file was completely not running due to a previous syntax error
j
started
a
Sweet they all pass now 🙂
j
@anil mind adding unit tests for the sync client and async client?
a
Sure, updated. It was a bit funky to set up the tests to work for both async and sync code. Let me know what you think
The benefit of my approach is you write the tests once and they'll run with all 4 clients. The downside is you have to add if is_async: resp = await resp to every call
j
ehh, that's fine
maybe wrap it in a function likje
async get_result(...)
and get_result just returns in the sync case?
not sure
a
Ah nice idea. I was able to auto-detect and clean it up. Looks a lot better now
j
running
lint failure
a
Fixed
j
another lint issue
a
Hopefully the last one. I added a few comments how to run these locally. In the future, you all might consider https://pre-commit.com/ to do this automatically
j
interesting
we've mostly pushed stuff like that into 
mage
in our Go repos: https://magefile.org/
a
Thanks for taking a look! Contributing to OSS isn't supposed to be this easy 😂
j
thanks for the contribution!
and it should be!
PreviousNext
Powered by