search

jackc / tern

The SQL Fan's Migrator
MIT License
925 stars 68 forks source link

Upgrade to pgx v4 #14

Closed alex closed 4 years ago

alex commented 4 years ago

Seems to mostly work -- there's two big TODOs in here, I'm curious if you have an opinion on what the right resolution is -- should I just copy the code as it used to exist in pgx?

jackc commented 4 years ago

The functionality of ParseEnvLibpq is baked now baked into ParseConfig. Parsing an empty connection string reads from the environment. So the usage in the LoadConfig function can simply be replaced with ParseConfig("").

The usage in *Config.Connect is slightly trickier. The fields that need to be copied would need to be updated (I think it would be remove UseFallbackTLS and FallbackTLSConfig and add Fallbacks.

I think this would be sufficient. But if you feel ambitious, there is a possible nice refactor. The whole multi-step way it is integrating the environment, CLI args, and config file complicates things and could be eliminated. The values from CLI arguments and the config file that are for pgx could be loaded into a map[string]string instead of directly into a pgx.ConnConfig. That map would be converted into a connection string (easy to do in the DSN format). Then that connection string could be passed into ParseConfig and the whole SslMode / ParseEnvLibpq in *Config.Connect could be removed.

alex commented 4 years ago

I think for now I'd like to do the sort of minimum changes required to get this passing, if that's all right with you.

I'll look into the things you suggested.

alex commented 4 years ago

Ok, integrated that.

jackc commented 4 years ago

Thanks!

alex commented 4 years ago

Thank you!