db.pg: Allow postgres connection using service definitions #19288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
medvednikov
reviewed
Sep 6, 2023
spytheman
reviewed
Sep 6, 2023
vlib/db/pg/pg.v
Outdated
Comment on lines
58
to
61
| pub struct Service { | ||
| pub: | ||
| name string | ||
| } |
There was a problem hiding this comment.
Why do you need a separate struct for this?
It contains a single string field, so why not just:
pub fn connect_with_conninfo(conninfo string) !DB {,
instead of the private fn do_connect(conninfo string) !DB {,
... then pub fn connect(config Config) !DB { can supply it with what is needed, and you can call it with just 'service=$xyz' .
There was a problem hiding this comment.
I was trying to be as much "statically typed" as possible. I agree this is much simpler. Updated.
spytheman
added a commit
to Spydr06/v
that referenced
this pull request
Sep 8, 2023
* master: (128 commits) ci: update containers_ci.yml to not try to install libexecinfo-static, which is not present on the latest Alpine image checker: disallow module name duplicates in local names (vlang#18118) cgen: cleanup go_before_stmt(0) (vlang#19308) parser: disallow using `sql` as name (vlang#19298) fmt: cleanup fmt comments (vlang#19306) tests: supplement test cases in fixed_array_const_size_test.v (vlang#19303) vdoc: prevent main-content outline with certain devices / browsers (vlang#19304) cgen: fix printing struct with thread field (vlang#19302) vlib: replace macros that resolve to __builtin_bswapnn calls for tcc (vlang#19305) time: add `MMM` in the doc comment for parse_format() (vlang#19299) os: include sys/sysctl.h on FreeBSD to avoid implicit definition of sysctl function (vlang#19293) parser, transformer: fix transformer.infix_expr() and cleanup parse_types.v (related vlang#19269) (vlang#19276) fmt: simplify the processing logic for removing inline comments (vlang#19297) db.pg: allow postgres connection using service definitions (vlang#19288) builtin: use `libgc-threaded` on FreeBSD, to get the threaded version of libgc (vlang#19294) ci: test the pure V math versions without .c.v overrides on the CI too (vlang#19292) all: support `./v -exclude @vlib/math/*.c.v vlib/math/math_test.v`, for using the pure V math module implementation, without the .c.v overrides there. (vlang#19290) .cirrus.yml: change test_zip_modules -> test_zip_modules_script, so that the CI can recognise the new task name and execute it thirdparty/zip: properly include utime.h and set defines for FreeBSD (vlang#19285) math: fix pure v math.pow (vlang#19287) ...
Wertzui123
pushed a commit
to Wertzui123/v
that referenced
this pull request
Oct 8, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support making the pg connection using service names that can be defined in
~/.pg_service.conf(docs). This immediately adds implicit support for all connection parameters the server supports, without adding fields topg.Configor needing to manually parse files.This is my first contribution and I'm new to vlang, so I'm not sure about best practices. Here are some considerations:
~/.pg_service.conffile set up. I'm happy to add this test if required, though.Servicestruct that will be accepted byconnect_with_service()but I could just let it accept astringinstead.service stringtoConfigand have it take precedence over all other attributes if it is non-empty.馃 Generated by Copilot at 6b93449
Added support for PostgreSQL service names in
pgmodule. Refactored connection logic into a separate function.馃 Generated by Copilot at 6b93449
Servicestruct to represent a PostgreSQL service name (link)connect_with_servicefunction to make a new connection using a service name (link)do_connectfunction to make a new connection using a connection string (link)