-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add a SocketPath type for linux systems
#10378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a SocketPath type for linux systems
#10378
Conversation
CodSpeed Performance ReportMerging #10378 will not alter performanceComparing Summary
|
|
Test will need to be skipped on windows; as it doesn't have sockets to begin with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should do something like:
Edit: Seems like on Windows, is_socket returns False, so maybe it's safe to not add these kind of conditions.
|
Looking good though, happy to support this once we get tests passing, etc :). |
|
ok for the issue with the Mac tests - pytest seems to be configured wrong, the temporary paths its generating with its fixture exceed the maximum length of a socket name on that platform. |
I'm ok only running the test on Linux. The goal isn't to test that creating sockets works, but to ensure that |
62fdf62 to
10d4ce8
Compare
10d4ce8 to
b874a82
Compare
|
Added condition to skip new test if not linux. |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
|
please review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks!
SocketPath typeSocketPath type for linux systems
|
@sydney-runkle when can i expect this change to be released? 😇 |
|
@theunkn0wn1, with our 2.10 release in late October! Feel free to install from main in the meantime :) |
Change Summary
The existing FilePath type does not accept sockets, as they fail the
Path.is_file(self)check.As such, I added a new
SocketPathtype and corresponding validator usingPath.is_socket(self).Related issue number
fix #10376
Checklist
Selected Reviewer: @sydney-runkle