search

paulscherrerinstitute / scicat-cli

Command line tools for interacting with the PSI SciCat data catalog
https://scicatproject.github.io/documentation/Ingestor/ingestManual.html
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

add common flags that are shared between commands (user, token, config) #89

Closed consolethinks closed 3 months ago

consolethinks commented 3 months ago

This commit moves two flags (user and token) and adds a (for now unused) config flag as persistent flags to the root command.

This means that the flags can be specified before or after the subcommand, and they don't have to be declared for each subcommand. The handling of them still happens in subcommands.

sbliven commented 3 months ago

I tried testing and get a segfault:

$ /Users/bliven_s/git/scicat-cli/dist/scicat-cli_darwin_amd64_v1/scicat-cli  --token $(op read $SCICAT_TOKEN_QA) datasetIngestor --testenv metadata.json
2024/07/18 15:41:38 Latest version: 2.2.2
2024/07/18 15:41:38 Your version of this program is up-to-date
2024/07/18 15:41:39 You are about to add a dataset to the === test === data catalog environment...
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x38 pc=0x13603a6]

goroutine 1 [running]:
github.com/paulscherrerinstitute/scicat/datasetUtils.GetUserInfoFromToken(0xc00028c101?, {0x1448e03, 0x1e}, {0x205b873fa, 0x41})
    github.com/paulscherrerinstitute/scicat/datasetUtils/getUserInfoFromToken.go:22 +0xe6
github.com/paulscherrerinstitute/scicat/datasetUtils.(*RealAuthenticator).GetUserInfoFromToken(0xc000207470?, 0x1083ad8?, {0x1448e03?, 0xc0002074c0?}, {0x205b873fa?, 0x176cc80?})
    github.com/paulscherrerinstitute/scicat/datasetUtils/authenticate.go:70 +0x2c
github.com/paulscherrerinstitute/scicat/datasetUtils.Authenticate({0x14f5568, 0x17a83c0}, 0x143d3c8?, {0x1448e03, 0x1e}, 0xc0002078c0, 0xc0002078b0)
    github.com/paulscherrerinstitute/scicat/datasetUtils/authenticate.go:47 +0x34b
github.com/paulscherrerinstitute/scicat/cmd/commands.glob..func4(0xc00020eb00?, {0xc000130840, 0x1, 0x143ce20?})
    github.com/paulscherrerinstitute/scicat/cmd/commands/datasetIngestor.go:169 +0xd2c
github.com/spf13/cobra.(*Command).execute(0x1773780, {0xc000130800, 0x4, 0x4})
    github.com/spf13/cobra@v1.8.0/command.go:987 +0xaa3
github.com/spf13/cobra.(*Command).ExecuteC(0x1774300)
    github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
    github.com/spf13/cobra@v1.8.0/command.go:1039
github.com/paulscherrerinstitute/scicat/cmd/commands.Execute()
    github.com/paulscherrerinstitute/scicat/cmd/commands/root.go:20 +0x1a
main.main()
    github.com/paulscherrerinstitute/scicat/cmd/main.go:6 +0xf
sbliven commented 3 months ago

The error is from an incorrect token (I had a trailing newline accidently). It should be fixed but it's not related to this PR.

consolethinks commented 3 months ago

I like the approach of making authentication options global. I also like how cobra handles them; for instance you can put --token either before or after the datasetIngestor subcommand.

I think more options should also be made global: --version; environment selection (--testenv/--devenv, which could be combined into --env=[dev|qa|test|production]); --noninteractive possibly. If these need more discussion they can be moved to a separate issue.

the environment switches are left in for now, but eventually I would like to replace them with some kind of config file. I'm generally not a fan of hard coded endpoints. However, the version idea is a good point and will add that to this PR.