feat: allow configuring default sort columns for each supported resource #795

phm07 · 2024-06-26T13:47:56Z

This PR adds options to configure the default sorting column for each resource that supports the list command.
See #418 and #434

phm07 · 2024-06-26T13:49:36Z

This is how the table that shows different options when using hcloud help config looks now:

┌───────────────────────┬──────────────────────┬─────────────┬───────────────────────┬──────────────────────────────┬─────────────────┐
│ OPTION                │ DESCRIPTION          │ TYPE        │ CONFIG KEY            │ ENVIRONMENT VARIABLE         │ FLAG            │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ debug                 │ Enable debug output  │ boolean     │ debug                 │ HCLOUD_DEBUG                 │ --debug         │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ debug-file            │ File to write debug  │ string      │ debug_file            │ HCLOUD_DEBUG_FILE            │ --debug-file    │
│                       │ output to            │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ default-ssh-keys      │ Default SSH keys for │ string list │ default_ssh_keys      │ HCLOUD_DEFAULT_SSH_KEYS      │                 │
│                       │ new servers          │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ endpoint              │ Hetzner Cloud API    │ string      │ endpoint              │ HCLOUD_ENDPOINT              │ --endpoint      │
│                       │ endpoint             │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ poll-interval         │ Interval at which to │ duration    │ poll_interval         │ HCLOUD_POLL_INTERVAL         │ --poll-interval │
│                       │ poll information,    │             │                       │                              │                 │
│                       │ for example action   │             │                       │                              │                 │
│                       │ progress             │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ quiet                 │ If true, only print  │ boolean     │ quiet                 │ HCLOUD_QUIET                 │ --quiet         │
│                       │ error messages       │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.certificate      │ Default sorting for  │ string list │ sort.certificate      │ HCLOUD_SORT.CERTIFICATE      │                 │
│                       │ Certificate resource │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.datacenter       │ Default sorting for  │ string list │ sort.datacenter       │ HCLOUD_SORT.DATACENTER       │                 │
│                       │ Datacenter resource  │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.firewall         │ Default sorting for  │ string list │ sort.firewall         │ HCLOUD_SORT.FIREWALL         │                 │
│                       │ Firewall resource    │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.floatingip       │ Default sorting for  │ string list │ sort.floatingip       │ HCLOUD_SORT.FLOATINGIP       │                 │
│                       │ Floating IP resource │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.image            │ Default sorting for  │ string list │ sort.image            │ HCLOUD_SORT.IMAGE            │                 │
│                       │ Image resource       │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.iso              │ Default sorting for  │ string list │ sort.iso              │ HCLOUD_SORT.ISO              │                 │
│                       │ ISO resource         │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.loadbalancer     │ Default sorting for  │ string list │ sort.loadbalancer     │ HCLOUD_SORT.LOADBALANCER     │                 │
│                       │ Load Balancer        │             │                       │                              │                 │
│                       │ resource             │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.loadbalancertype │ Default sorting for  │ string list │ sort.loadbalancertype │ HCLOUD_SORT.LOADBALANCERTYPE │                 │
│                       │ Load Balancer Type   │             │                       │                              │                 │
│                       │ resource             │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.location         │ Default sorting for  │ string list │ sort.location         │ HCLOUD_SORT.LOCATION         │                 │
│                       │ Location resource    │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.network          │ Default sorting for  │ string list │ sort.network          │ HCLOUD_SORT.NETWORK          │                 │
│                       │ Network resource     │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.placementgroup   │ Default sorting for  │ string list │ sort.placementgroup   │ HCLOUD_SORT.PLACEMENTGROUP   │                 │
│                       │ Placement Group      │             │                       │                              │                 │
│                       │ resource             │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.primaryip        │ Default sorting for  │ string list │ sort.primaryip        │ HCLOUD_SORT.PRIMARYIP        │                 │
│                       │ Primary IP resource  │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.server           │ Default sorting for  │ string list │ sort.server           │ HCLOUD_SORT.SERVER           │                 │
│                       │ Server resource      │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.servertype       │ Default sorting for  │ string list │ sort.servertype       │ HCLOUD_SORT.SERVERTYPE       │                 │
│                       │ Server Type resource │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.sshkey           │ Default sorting for  │ string list │ sort.sshkey           │ HCLOUD_SORT.SSHKEY           │                 │
│                       │ SSH Key resource     │             │                       │                              │                 │
├───────────────────────┼──────────────────────┼─────────────┼───────────────────────┼──────────────────────────────┼─────────────────┤
│ sort.volume           │ Default sorting for  │ string list │ sort.volume           │ HCLOUD_SORT.VOLUME           │                 │
│                       │ Volume resource      │             │                       │                              │                 │
└───────────────────────┴──────────────────────┴─────────────┴───────────────────────┴──────────────────────────────┴─────────────────┘

Is this too much? Or should we keep all the entries fort sort.[resource]?

codecov · 2024-06-26T13:50:06Z

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 59.47%. Comparing base (320300c) to head (c5f5823).

Files	Patch %	Lines
internal/cmd/base/list.go	58.33%	2 Missing and 3 partials ⚠️
internal/state/config/options.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #795      +/-   ##
==========================================
- Coverage   59.60%   59.47%   -0.14%     
==========================================
  Files         210      210              
  Lines        7655     7667      +12     
==========================================
- Hits         4563     4560       -3     
- Misses       2450     2458       +8     
- Partials      642      649       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jooola · 2024-06-26T13:53:34Z

~~Do you think it's possible to decouple the SortOption and the per resource sort arguments? Similar to the tables columns?~~

EDIT: Scratch that.

jooola · 2024-06-26T15:00:48Z

Is this too much? Or should we keep all the entries fort sort.[resource]?

I'd rather keep it explicit, unless we have a clear list of resource names.

sort.loadbalancertype │ HCLOUD_SORT.LOADBALANCERTYPE

Could we keep the _ or - in the names? The env variable name is not the final ones right?

phm07 · 2024-06-27T09:48:55Z

sort.loadbalancertype │ HCLOUD_SORT.LOADBALANCERTYPE

Could we keep the _ or - in the names? The env variable name is not the final ones right?

We could either:

Create an override for the environment variable names (which would introduce inconsistency)
Or just rename the options to include dashes, for example sort.servertype -> sort.server-type. This would lead to the config key name being sort.server_type and the environment variable being HCLOUD_SORT_SERVER_TYPE.

It's a style question, I don't like mixing dots and dashes for the key, but I think it would be best for consistency and readability if we did.

Edit: I just noticed that the environment variables contain dots. That's a bug/oversight. I don't think env vars support dots in their names.

jooola · 2024-06-27T09:58:45Z

Nope, env vars cannot have dots:

Environment variable names used by the utilities in the Shell and Utilities volume of POSIX.1-2017 consist solely of uppercase letters, digits, and the ( '_' ) from the characters defined in Portable Character Set and do not begin with a digit. Other characters may be permitted by an implementation; applications shall tolerate the presence of such names. Uppercase and lowercase letters shall retain their unique identities and shall not be folded together. The name space of environment variable names containing lowercase letters is reserved for applications. Applications can define any environment variables with names from this name space without modifying the behavior of the standard utilities.

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html

apricote · 2024-06-27T14:33:12Z

Without taking a look at the code:

I think the options should be the same as the subcommands: hcloud load-balancer -> sort.load-balancer
In the docs I would prefer to have a single element sort.<resource> to keep this reasonably small.
I would add a section in hcloud config --help that describes these options in general

phm07 · 2024-07-03T13:07:58Z

Without taking a look at the code:

I think the options should be the same as the subcommands: hcloud load-balancer -> sort.load-balancer

In the docs I would prefer to have a single element sort.<resource> to keep this reasonably small.

I would add a section in hcloud config --help that describes these options in general

Agreed. Done in 0261566 and 9b23dfc

apricote

🚀

apricote · 2024-07-04T12:20:18Z

While testing this, I found that hcloud config add adds to the front of the list. I found this counterintuitive. I wanted to set a default for hcloud image list --sort name:asc --sort created:asc so I ran:

$ hcloud config add sort.image name:asc
$ hcloud config add sort.image created:asc

Thinking it would end us as [name:asc created:asc]. Turned out it was the other way around, and running hcloud image list now orders the other way around.

$ hcloud config get sort.image
[created:asc name:asc]

So feature works technically, but the UX was unexpected

apricote · 2024-07-04T12:22:03Z

Even with hcloud config set sort.image name:asc created:asc its being saved in the wrong order.

phm07 · 2024-07-04T16:32:03Z

Good catch! #805 fixes this. We should wait for #805 before we merge this PR.

apricote

I noticed that this PR does not compile anymore because of #798. Could you rebase this PR and fix the compilation error?

The functionality works great now!

internal/state/config/options.go

apricote

🚀

🤖 I have created a release *beep* *boop* --- ## [1.45.0](v1.44.2...v1.45.0) (2024-07-17) ### Features * allow configuring default sort columns for each supported resource ([#795](#795)) ([f6877a1](f6877a1)) * better error messages on malformed option value ([#798](#798)) ([8c6fec9](8c6fec9)) ### Bug Fixes * **config:** ordering of list option values not preserved ([#805](#805)) ([1ac27bf](1ac27bf)) * debug log is truncated if it already exists ([#793](#793)) ([c3d3a9f](c3d3a9f)) * **firewall:** wrong wording when firewall is removed from resource ([#812](#812)) ([9017a65](9017a65)), closes [#809](#809) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

phm07 added the feature label Jun 26, 2024

phm07 self-assigned this Jun 26, 2024

phm07 requested a review from a team as a code owner June 26, 2024 13:47

phm07 force-pushed the default-sort-column-options branch from adafb8c to 513372e Compare June 27, 2024 15:29

apricote approved these changes Jul 3, 2024

View reviewed changes

apricote requested changes Jul 5, 2024

View reviewed changes

internal/state/config/options.go Outdated Show resolved Hide resolved

phm07 added 3 commits July 10, 2024 13:26

feat: allow configuring default sort columns for each resource

80cd05d

rename options

138b895

adjust help text

dedc654

phm07 mentioned this pull request Jul 10, 2024

allow sorting to be configured globally (closes #418) #421

Closed

fix after rebase

23ce28c

phm07 force-pushed the default-sort-column-options branch from 9b23dfc to 23ce28c Compare July 10, 2024 11:41

phm07 added 2 commits July 10, 2024 15:38

remove options for unsupported resources

5edb185

add warning message

c5f5823

phm07 changed the title ~~feat: allow configuring default sort columns for each resource~~ feat: allow configuring default sort columns for each supported resource Jul 10, 2024

apricote approved these changes Jul 10, 2024

View reviewed changes

phm07 merged commit f6877a1 into main Jul 10, 2024
5 checks passed

phm07 deleted the default-sort-column-options branch July 10, 2024 15:52

hcloud-bot mentioned this pull request Jul 8, 2024

chore(main): release 1.45.0 #804

Merged

This was referenced Jul 11, 2024

Feature request: allow sorting to be configured globally #418

Closed

feat: Additional Settings (e.g. sort preference, default columns) #434

Closed

BrewTestBot mentioned this pull request Jul 17, 2024

hcloud 1.45.0 Homebrew/homebrew-core#177594

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow configuring default sort columns for each supported resource #795

feat: allow configuring default sort columns for each supported resource #795

phm07 commented Jun 26, 2024

phm07 commented Jun 26, 2024

codecov bot commented Jun 26, 2024 •

edited

Loading

jooola commented Jun 26, 2024 •

edited

Loading

jooola commented Jun 26, 2024

phm07 commented Jun 27, 2024 •

edited

Loading

jooola commented Jun 27, 2024 •

edited

Loading

apricote commented Jun 27, 2024

phm07 commented Jul 3, 2024

apricote left a comment

apricote commented Jul 4, 2024

apricote commented Jul 4, 2024

phm07 commented Jul 4, 2024

apricote left a comment

apricote left a comment

feat: allow configuring default sort columns for each supported resource #795

feat: allow configuring default sort columns for each supported resource #795

Conversation

phm07 commented Jun 26, 2024

phm07 commented Jun 26, 2024

codecov bot commented Jun 26, 2024 • edited Loading

Codecov Report

jooola commented Jun 26, 2024 • edited Loading

jooola commented Jun 26, 2024

phm07 commented Jun 27, 2024 • edited Loading

jooola commented Jun 27, 2024 • edited Loading

apricote commented Jun 27, 2024

phm07 commented Jul 3, 2024

apricote left a comment

Choose a reason for hiding this comment

apricote commented Jul 4, 2024

apricote commented Jul 4, 2024

phm07 commented Jul 4, 2024

apricote left a comment

Choose a reason for hiding this comment

apricote left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 26, 2024 •

edited

Loading

jooola commented Jun 26, 2024 •

edited

Loading

phm07 commented Jun 27, 2024 •

edited

Loading

jooola commented Jun 27, 2024 •

edited

Loading