Add `--database` flag to `scope` and `psql` by abrightwell · Pull Request #5 · CrunchyData/bridge-cli

abrightwell · 2022-01-18T20:45:24Z

These changes add the ability to specify a database name to connect when
using the scope and psql command.

The flag is setup to be optional. If not provided then it will default
to postgres.

> ./bin/cb psql --help
Usage: cb psql <cluster id> [--database] [-- [args for psql such as -c or -f]]
    -h, --help                       Show this help
    --database NAME                  Database name (default: postgres)

> ./bin/cb scope --help
Usage: cb scope <--cluster> [--(check),...]
    -h, --help                       Show this help
...
    --database NAME                  Database name (default: postgres)
...

Example with psql:

> ./bin/cb psql <test-cluster-id>
connecting to personal/test-cluster
psql (14.1 (Ubuntu 14.1-1.pgdg18.04+1), server 13.4)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, bits: 256, compression: off)
Type "help" for help.

personal/test-cluster/postgres=# \q

> ./bin/cb psql <test-cluster-id> --database foo
connecting to personal/test-cluster
psql (14.1 (Ubuntu 14.1-1.pgdg18.04+1), server 13.4)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, bits: 256, compression: off)
Type "help" for help.

personal/test-cluster/foo=# \q

spec/cb/completion_spec.cr

src/cb/psql.cr

will

Could you please also add completion and completion specs, and a changelog entry?

will · 2022-01-24T15:05:41Z

src/cb/psql.cr

  end

+  def database=(str : String)
+    str = str.downcase


Do we want this? I don’t know what happens if people have upper case characters in their database name with postgres at all.

So, yeah, likely not necessary. Will update.

will · 2022-01-24T15:07:55Z

src/cli.cr


  parser.on("psql", "Connect to the database using `psql`") do
-    parser.banner = "Usage: cb psql <cluster id> [-- [args for psql such as -c or -f]]"
+    parser.banner = "Usage: cb psql <cluster id> [<--database>] [-- [args for psql such as -c or -f]]"


Suggested change

parser.banner = "Usage: cb psql <cluster id> [<--database>] [-- [args for psql such as -c or -f]]"

parser.banner = "Usage: cb psql <cluster id> [--database] [-- [args for psql such as -c or -f]]"

I've been going with [] is for optional and <> for required, so I think we just need the []

will · 2022-01-24T15:18:54Z

src/cb/psql.cr

+    if database.presence
+      uri.path = database.to_s
+    end


This is the same reason as the case in the other PR with the instance variable still being a union with nil even in the conditional. It is technically possible for some thread to change the value of @database (which these two call to #database is returning) to change between calls. For this cli program, it'll never happen, but that's why the call to #presence really isn't doing much, and why I think probably the complier made you tack on #to_s.

One way would be

Suggested change

if database.presence

uri.path = database.to_s

end

if (db = database)

uri.path = db

end

But also you could do it I think like this, which is nice because the above creates a local variable that is in scope for the rest of the method, whereas the block argument is only in scope in the block.

Suggested change

if database.presence

uri.path = database.to_s

end

database.tap { |db| uri.path = db if db }

I don’t think it matters a whole lot either way. Just explaining all the options and what's going on assuming you're new to Crystal.

These changes add the ability to specify a database name to connect when using the `scope` and `psql` command. The flag is setup to be optional. If not provided then it will default to `postgres`.

``` <<< /nix/store/6sk9jhxaqa01jm0wp0b4x9labcyi5a3c-cb-3.6.7.drv >>> /nix/store/jwxavs1iq4zp0bshivkz508js1s8f6g2-cb-3.6.7.drv Version changes: [C.] #1 binutils-with-gold 2.44.tar.bz2 x2 -> 2.44.tar.bz2 [C.] #2 builder.pl <none> x3 -> <none> x2 [C.] #3 cctools-binutils-darwin-wrapper 1010.6 x7 -> 1010.6 x5 [C.] #4 clang 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #5 clang-at-least 16-LLVMgold-path.patch x4 -> 16-LLVMgold-path.patch x3 [C.] #6 clang-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #7 clang-wrapper 20.1.8 x3, 21.1.2 x3 -> 21.1.2 x3 [C.] #8 compiler-rt 20.1.8, 21.1.2 -> 21.1.2 [C.] #9 compiler-rt-libc 20.1.8, 21.1.2 -> 21.1.2 [C.] #10 compiler-rt-src 20.1.8, 21.1.2 -> 21.1.2 [C.] #11 cpio 2.15 x3, 2.15.tar.bz2 x2 -> 2.15 x2, 2.15.tar.bz2 [C*] #12 crystal 1.10.1-1-darwin-universal.tar.gz, 1.16.3 -> 1.10.1-1-darwin-universal.tar.gz, 1.18.2 [C.] #13 expand-response-params <none> x5 -> <none> x4 [C.] #14 gnu-install-dirs.patch <none> x4 -> <none> x3 [C.] #15 jq 1.8.1 x2, 1.8.1.tar.gz x2 -> 1.8.1, 1.8.1.tar.gz [C.] #16 libbfd-plugin-api-header <none> x3 -> <none> x2 [C.] #17 libcxx 19.1.2+apple-sdk-15.5 x5 -> 19.1.2+apple-sdk-15.5 x4 [C.] #18 llvm 18-compatibility.patch, 20.1.8, 21.1.2 x2 -> 18-compatibility.patch, 21.1.2 x2 [C.] #19 llvm-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #20 llvm-tblgen 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #21 llvm-tblgen-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #22 macOS-SDK 11.3 x2, 14.4, 15.5 x3 -> 11.3 x2, 14.4, 15.5 x2 [C.] #23 make-binary-wrapper-hook <none> x4 -> <none> x3 [C.] #24 onig 6.9.10.tar.gz x2 -> 6.9.10.tar.gz [C.] #25 oniguruma 6.9.10 x2 -> 6.9.10 [C.] #26 pbzx 1.0.2 x3 -> 1.0.2 x2 [C.] #27 psutil 7.1.0.tar.gz x2 -> 7.1.0.tar.gz [C.] #28 python3 3.13.8 x2, 3.13.8-env x2 -> 3.13.8 x2, 3.13.8-env [C.] #29 python3.13-psutil 7.1.0 x2 -> 7.1.0 [C*] #30 source <none> x85 -> <none> x83 [C*] #31 stdenv-darwin <none> x5 -> <none> x3 Removed packages: [R.] #1 bd49bbaaafc98433a2cb4e95ce25b7a201baf5a5.patch <none> Closure size: 1239 -> 1201 (806 paths added, 844 paths removed, delta -38, disk usage -161.7KiB). ```

* Allow temp location to change if necessary To keep the specs completely isolated, we don't want to reuse the same global tempdir. * update crystal 1.16 -> 1.18 ``` <<< /nix/store/6sk9jhxaqa01jm0wp0b4x9labcyi5a3c-cb-3.6.7.drv >>> /nix/store/jwxavs1iq4zp0bshivkz508js1s8f6g2-cb-3.6.7.drv Version changes: [C.] #1 binutils-with-gold 2.44.tar.bz2 x2 -> 2.44.tar.bz2 [C.] #2 builder.pl <none> x3 -> <none> x2 [C.] #3 cctools-binutils-darwin-wrapper 1010.6 x7 -> 1010.6 x5 [C.] #4 clang 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #5 clang-at-least 16-LLVMgold-path.patch x4 -> 16-LLVMgold-path.patch x3 [C.] #6 clang-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #7 clang-wrapper 20.1.8 x3, 21.1.2 x3 -> 21.1.2 x3 [C.] #8 compiler-rt 20.1.8, 21.1.2 -> 21.1.2 [C.] #9 compiler-rt-libc 20.1.8, 21.1.2 -> 21.1.2 [C.] #10 compiler-rt-src 20.1.8, 21.1.2 -> 21.1.2 [C.] #11 cpio 2.15 x3, 2.15.tar.bz2 x2 -> 2.15 x2, 2.15.tar.bz2 [C*] #12 crystal 1.10.1-1-darwin-universal.tar.gz, 1.16.3 -> 1.10.1-1-darwin-universal.tar.gz, 1.18.2 [C.] #13 expand-response-params <none> x5 -> <none> x4 [C.] #14 gnu-install-dirs.patch <none> x4 -> <none> x3 [C.] #15 jq 1.8.1 x2, 1.8.1.tar.gz x2 -> 1.8.1, 1.8.1.tar.gz [C.] #16 libbfd-plugin-api-header <none> x3 -> <none> x2 [C.] #17 libcxx 19.1.2+apple-sdk-15.5 x5 -> 19.1.2+apple-sdk-15.5 x4 [C.] #18 llvm 18-compatibility.patch, 20.1.8, 21.1.2 x2 -> 18-compatibility.patch, 21.1.2 x2 [C.] #19 llvm-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #20 llvm-tblgen 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #21 llvm-tblgen-src 20.1.8, 21.1.2 x2 -> 21.1.2 x2 [C.] #22 macOS-SDK 11.3 x2, 14.4, 15.5 x3 -> 11.3 x2, 14.4, 15.5 x2 [C.] #23 make-binary-wrapper-hook <none> x4 -> <none> x3 [C.] #24 onig 6.9.10.tar.gz x2 -> 6.9.10.tar.gz [C.] #25 oniguruma 6.9.10 x2 -> 6.9.10 [C.] #26 pbzx 1.0.2 x3 -> 1.0.2 x2 [C.] #27 psutil 7.1.0.tar.gz x2 -> 7.1.0.tar.gz [C.] #28 python3 3.13.8 x2, 3.13.8-env x2 -> 3.13.8 x2, 3.13.8-env [C.] #29 python3.13-psutil 7.1.0 x2 -> 7.1.0 [C*] #30 source <none> x85 -> <none> x83 [C*] #31 stdenv-darwin <none> x5 -> <none> x3 Removed packages: [R.] #1 bd49bbaaafc98433a2cb4e95ce25b7a201baf5a5.patch <none> Closure size: 1239 -> 1201 (806 paths added, 844 paths removed, delta -38, disk usage -161.7KiB). ``` --------- Co-authored-by: Will Leinweber <will@bitfission.com>

abrightwell commented Jan 18, 2022

View reviewed changes

spec/cb/completion_spec.cr Outdated Show resolved Hide resolved

src/cb/psql.cr Outdated Show resolved Hide resolved

abrightwell requested a review from will January 18, 2022 20:47

abrightwell force-pushed the database-name branch 2 times, most recently from 416fc7b to 34406f0 Compare January 18, 2022 20:57

will requested changes Jan 24, 2022

View reviewed changes

abrightwell force-pushed the database-name branch 2 times, most recently from a278488 to c155fe3 Compare January 25, 2022 21:13

Add --database flag to scope and psql

a9fd349

These changes add the ability to specify a database name to connect when using the `scope` and `psql` command. The flag is setup to be optional. If not provided then it will default to `postgres`.

abrightwell force-pushed the database-name branch from c155fe3 to a9fd349 Compare January 25, 2022 21:15

abrightwell requested a review from will January 25, 2022 21:15

will merged commit a9fd349 into CrunchyData:main Jan 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `--database` flag to `scope` and `psql`#5

Add `--database` flag to `scope` and `psql`#5
will merged 1 commit intoCrunchyData:mainfrom
abrightwell:database-name

abrightwell commented Jan 18, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

will left a comment

Uh oh!

will Jan 24, 2022

Uh oh!

abrightwell Jan 25, 2022 •

edited

Loading

Uh oh!

will Jan 24, 2022

Uh oh!

will Jan 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	parser.banner = "Usage: cb psql <cluster id> [<--database>] [-- [args for psql such as -c or -f]]"
	parser.banner = "Usage: cb psql <cluster id> [--database] [-- [args for psql such as -c or -f]]"

Conversation

abrightwell commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

will left a comment

Choose a reason for hiding this comment

Uh oh!

will Jan 24, 2022

Choose a reason for hiding this comment

Uh oh!

abrightwell Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

will Jan 24, 2022

Choose a reason for hiding this comment

Uh oh!

will Jan 24, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abrightwell commented Jan 18, 2022 •

edited

Loading

abrightwell Jan 25, 2022 •

edited

Loading