fly-apps / fly-apps/postgres-importer

Defaults are maybe a little confusing/dangerous?

Open
#5 3 comments 1 reaction 0 assignees View on GitHub
Dominant language
Shell
Stars
10
Forks
3
PR merge metrics
No merged PRs in 30d

Description

Hey, thanks for the great project, just wanted to offer some feedback on the CLI defaults.

```go
noOwner := flag.Bool("no-owner", true, "")
clean := flag.Bool("clean", true, "")
create := flag.Bool("create", true, "")
dataOnly := flag.Bool("data-only", false, "")
```

The default behaviour for pg_dump is to include ownership, so defaulting to `--no-owner=true` is a little surprising and can break RLS policies/security. Also supporting `--no-owner` but not `--owner` (for the reverse behavior also is a little confusing. It took me a while to realise that I could pass `=value` to override the default. As all CLI's have their own conventions, any surprises that can be avoided if possible as trial and error isn't great when doing prod migrations.

`clean` being defaulted to true is also a little dangerous. For example. what happens if a dev writes to the wrong database by mistake. Perhaps they reversed the target/source db uri accidentally (hey it happens!). In that case they are now potentially wiping production db objects despite not explicitly telling the tool to do so (because clean defaults to true).

I'd argue a tool like this should have no defaults. This is exactly the time when clarity should be prioritized over concision or convenience.

Final point, importing just a db is fine, as long as you also bring along db roles. To not bring along db roles and only database objects will only work for projects that use postgres as a simple store. It's a good practice for each service to have its own db role scoped to its level of access. It's also super simple to extract roles via pg_dumpall, so I'd recommend including roles as an option as `--no-owner=false` won't work if the owners don't exist.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.