-
-
Notifications
You must be signed in to change notification settings - Fork 82
Add support for UDS and existing Channels #335
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
Conversation
/cc @MahdiBM @tkrajacic |
Resolves #259 |
CI failure on |
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.
LGTM - only thing to mention that I couldn't quite tell is that it looks like all the tests have been updated to use the new APIs. We should probably have a test just to ensure the deprecated stuff still works
e407648
to
f77602e
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #335 +/- ##
==========================================
- Coverage 41.27% 41.04% -0.23%
==========================================
Files 115 117 +2
Lines 9572 9657 +85
==========================================
+ Hits 3951 3964 +13
- Misses 5621 5693 +72
|
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.
Looks way better.
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
6eca626
to
7f950f6
Compare
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.
LOVELY! Nits
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
Sources/PostgresNIO/Connection/PostgresConnection+Configuration.swift
Outdated
Show resolved
Hide resolved
…socket or an already-established NIO Channel
…zed types, and move the config stuff its own source file 'cause it got to be too long.
…able (falls back to the BUILDING_DOCC flag for older versions)
…rty, ditch pointless use of @_implementationOnly, use internal backing store rather than public API for internal access to config info, revert unnecessary changes to tests
7f950f6
to
e8ad7df
Compare
28b511f
to
f8ae7a2
Compare
f8ae7a2
to
d6edb3a
Compare
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.
Lovely! Thanks so much!
Extends
PostgresConnection
to enable connecting to Postgres servers over UNIX domain sockets or using a preexistingChannel
.In the process, most of the existing structure of
PostgresConnection.Configuration
has been deprecated in favor of new APIs with more options and cleaner design. The deprecation messages provide instructions on migrating existing code. List of deprecations and their replacements (all listed APIs should be assumed to have aPostgresConnection.
prefix):Configuration.Connection
and.connection
->Configuration.host
,.port
,.unixDomainSocket
, and.establishedChannel
properties, and respective initializers.Configuration.Authentication
and.authentication
->Configuration.username
,.password
, and.database
properties.Configuration.Options
structure and.options
property, containing theconnectTimeout
andrequireBackendKeyData
properties previously found onConfiguration.Connection
.Note: The integration tests for UNIX domain sockets will be skipped unless
POSTGRES_SOCKET
is set to a socket path in the test environment. See the updatedtest.yml
for examples of how to do so correctly.Closes #259
Closes #268