fix(core): fix for Windows platform

python-zk / kazoo

Kazoo is a high-level Python library that makes it easier to use Apache Zookeeper.

https://kazoo.readthedocs.io

Apache License 2.0

1.3k stars 386 forks source link

fix(core): fix for Windows platform #681

Closed StephenSorriaux closed 8 months ago

StephenSorriaux commented 1 year ago

Fixes https://github.com/python-zk/kazoo/issues/679

Why is this needed?

This fixes an issue with the Windows platform that appeared in the latest 2.9.0 release. This also adds a very basic testing for the Windows platform (only 1 job, for the latest version of Python and the latest version of ZK).

Proposed Changes

remove the os.fstat(fd) check that is invalid on Windows platform
adjust test_selector_select test
add a single job to test Windows platform (gevent seems broken on Windows so... skipped it): only 1 job because I don't feel like we need to re-test every version of Python/ZK for Windows

Does this PR introduce any breaking change?

Nah.

ceache commented 1 year ago

@StephenSorriaux is this a draft or ready for review yet?

StephenSorriaux commented 1 year ago

@ceache Not really, it is still a draft. I'm just too used to gitlab, which auto marks a PR as draft if "draft" is in the title.

jeffwidman commented 1 year ago

What's needed to move this forward? It'd be nice to get this resolved for our Windows users.

StephenSorriaux commented 1 year ago

I was waiting for feedbacks from @JetDrag / @ceache but I will go ahead and just finish it so that it can be reviewed.

codecov[bot] commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (d983066) 96.31% compared to head (e5becc3) 96.82%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #681 +/- ## ========================================== + Coverage 96.31% 96.82% +0.50% ========================================== Files 27 27 Lines 3556 3557 +1 ========================================== + Hits 3425 3444 +19 + Misses 131 113 -18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JetDrag commented 1 year ago

Excellent work！ @StephenSorriaux

StephenSorriaux commented 8 months ago

@ceache @jeffwidman Sorry to resurrect this only now, would appreciate another review if possible and finally merge this.

StephenSorriaux commented 8 months ago

Thanks for always being around my dear Jeff!