Skip to content

Add --version flag to create command.#8

Closed
abrightwell wants to merge 1 commit intoCrunchyData:mainfrom
abrightwell:abrightwell/br-890
Closed

Add --version flag to create command.#8
abrightwell wants to merge 1 commit intoCrunchyData:mainfrom
abrightwell:abrightwell/br-890

Conversation

@abrightwell
Copy link
Member

@abrightwell abrightwell commented Jan 20, 2022

This flag allows for specifying the postgres major version to the create
command. It is currently set to default to version 13.

Example:

> ./bin/cb create -p aws -r us-east-1 --plan hobby-2 --team <team_id> --version 14
...

> ./bin/cb info <cluster_id>
personal/Cluster 2022-01-20 14_09_30
     state: ready
   created: 2022-01-20T14:09:30Z
      plan: hobby-2 (2GiB ram, 1vCPU)
   version: 14
   storage: 100GiB
...

@abrightwell abrightwell requested a review from will January 20, 2022 14:36
Copy link
Collaborator

@will will left a comment

Choose a reason for hiding this comment

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

There isn't tab completion for the flag. It's a bit of a daunting section through, so I did this one as an example cafe18a

I think you can't pick a different version for a replica so I've excluded the suggestion if the replica field was set.

Could you also please add a changelog?

self.storage ||= source.storage
self.plan ||= source.plan_id
else
self.postgres_version ||= 13
Copy link
Collaborator

Choose a reason for hiding this comment

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

the api call to create will always allow a nil postgres_version, right? If that's the case, then I think we should not force a default here. That way when we decide to change the default, we don’t also need to release a new CLI.

Copy link
Member Author

Choose a reason for hiding this comment

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

The API does not default to any version, unfortunately. There was a deprecation of major_version in favor of postgres_version_id. For backwards compat, if the latter was omitted, it would default to the former. Current version of the CLI assigns statically to 13. Though, that's not an argument not to default in the API, which seems like a reasonable improvement to introduce. @brandur thoughts on setting a default major version?

Copy link
Contributor

Choose a reason for hiding this comment

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

WFM.

We already do kind of have a default in that there's one in there for Heroku in case someone doesn't passes any flags to heroku addons:add. We may want to expand on this and make this a global thing which would allow you to not pass any postgres_version_id to "cluster create", making it a bit easier to call.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I suppose I just assumed there was a default. It should be 14 now that 14.1 has been released

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

This change is deployed and Postgres 14 is now the Platform-wide default.

src/cli.cr Outdated
parser.on("-n NAME", "--name NAME", "Cluster name (default: Cluster date+time)") { |arg| create.name = arg }
parser.on("-p NAME", "--platform NAME", "Cloud provider") { |arg| create.platform = arg }
parser.on("-r NAME", "--region NAME", "Region/Location") { |arg| create.region = arg }
parser.on("-v VERSION", "--version VERSION", "Postgres version (default: 13)") { |arg| create.postgres_version = arg }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe should say it's a major version to discourage people from trying 14.1

Copy link
Contributor

Choose a reason for hiding this comment

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

+1.

This flag allows for specifying the postgres major version to the create
command. It is currently set to default to version 13.
suggest << "--ha\thigh availability" unless has_full_flag?(:ha) || has_full_flag?(:replica)
suggest << "--name\tcluster name" unless has_full_flag? :name
suggest << "--network\tnetwork id" unless has_full_flag? :network
suggest << "--version\tmajor version" unless has_full_flag?(:version) || has_full_flag?(:fork) || has_full_flag?(:replica)
Copy link
Member Author

@abrightwell abrightwell Jan 25, 2022

Choose a reason for hiding this comment

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

I double checked, neither fork nor replica allow for a version to be specified via the API

@abrightwell abrightwell requested a review from will January 25, 2022 20:10
@will
Copy link
Collaborator

will commented Jan 27, 2022

This is merged but I did the rebase on my computer so gh didn't figure it out

@will will closed this Jan 27, 2022
@abrightwell abrightwell deleted the abrightwell/br-890 branch April 4, 2022 12:34
will added a commit that referenced this pull request Oct 31, 2025
```
<<< /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).
```
will added a commit that referenced this pull request Oct 31, 2025
* 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>
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.

3 participants