Skip to content

Clean up and expose physical connection initializers#4678

Merged
roji merged 1 commit intonpgsql:mainfrom
roji:ConnectionInitializers
Oct 15, 2022
Merged

Clean up and expose physical connection initializers#4678
roji merged 1 commit intonpgsql:mainfrom
roji:ConnectionInitializers

Conversation

@roji
Copy link
Member

@roji roji commented Sep 26, 2022

@NinoFloris this one's for you.

Note that during the physical initialization phase, there's no multiplexing - the connection is bound to the connector (this is kinda obvious since the whole point is to do initialization on a physical connection).

The relationalship between connection and connector is ever annoying... Oh well.

Closes #3817
Closes #3745

@roji roji requested a review from NinoFloris September 26, 2022 09:42
@roji roji requested a review from vonzshik as a code owner September 26, 2022 09:42
@roji roji force-pushed the ConnectionInitializers branch from 91173fa to 9400c48 Compare September 26, 2022 09:48
@roji
Copy link
Member Author

roji commented Sep 26, 2022

BTW I did have one thought when doing this - we could allow several separate connection initializers to be added, and "aggregate" them into a single function. This would mainly mean changing the NpgsqlDataSourceBuilder API (e.g. AddConnectionInitializer, ResetConnectionInitializer).

But this is such a fringe API that it seems really overkill.

@roji roji force-pushed the ConnectionInitializers branch 2 times, most recently from bdfa19c to 95366ac Compare October 3, 2022 09:15
@roji roji mentioned this pull request Oct 3, 2022
@roji roji force-pushed the ConnectionInitializers branch from 95366ac to 1bd6d36 Compare October 3, 2022 10:33
@roji roji requested a review from vonzshik October 5, 2022 12:51
Copy link
Member

@NinoFloris NinoFloris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some naming feedback, otherwise LGTM

}
}

internal void MakeDisposed()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yum

@roji roji force-pushed the ConnectionInitializers branch from a3ed18b to edc9c63 Compare October 15, 2022 13:30
@roji roji enabled auto-merge (squash) October 15, 2022 13:31
@roji roji merged commit 4ea5d31 into npgsql:main Oct 15, 2022
@roji roji deleted the ConnectionInitializers branch October 15, 2022 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up and expose physical connection initializers Validate both physical connection initializers are either set or both are not

3 participants