Skip to content

Conversation

@mabdh
Copy link
Member

@mabdh mabdh commented Aug 15, 2022

Tasks

  • Error handling when cli config not found, for api that does not need config, could still do the call, otherwise return warning
  • Loading config for client init, pick from the root, but the flag could override it
  • Loading protos from proton based on a commit in Makefile
  • Namespacing server commands
  • Init for server config

Error handling in client config
The existence of $ODPF_CONFIG_DIR/shield.yaml

  • If user does not have this file in the path
    • calling shield client cli without --host would throw error need a config or flag --host
    • calling shield client cli that not requires auth
      • with --host would pass
    • calling shield cli that requires auth
      • with --host would return --header is required (we mark this as required)
      • with --host and --header would pass

Some changes in cli

+ shield config init // initializing client config
+ shield config list // list down client configurations
+ shield server init // initializing server config

- shield serve
+ shield server start

- shield migrate
+ shield server migrate

- shield migrate-rollback
+ shield server migrate-rollback

@mabdh mabdh linked an issue Aug 15, 2022 that may be closed by this pull request
@coveralls
Copy link

coveralls commented Aug 15, 2022

Pull Request Test Coverage Report for Build 2887124987

  • 319 of 482 (66.18%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-1.7%) to 49.195%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/client.go 19 23 82.61%
cmd/user.go 15 24 62.5%
cmd/action.go 15 25 60.0%
cmd/context.go 0 10 0.0%
cmd/group.go 14 24 58.33%
cmd/namespace.go 14 24 58.33%
cmd/policy.go 15 25 60.0%
cmd/project.go 15 25 60.0%
cmd/role.go 15 25 60.0%
cmd/organization.go 20 39 51.28%
Totals Coverage Status
Change from base Build 2866460769: -1.7%
Covered Lines: 3454
Relevant Lines: 7021

💛 - Coveralls

@mabdh mabdh force-pushed the cli-config-improvement branch from 5d0ff0e to 57977d9 Compare August 18, 2022 04:25
@mabdh mabdh force-pushed the cli-config-improvement branch from 57977d9 to 0bb6b7d Compare August 18, 2022 04:29
@mabdh mabdh changed the title feat: config management and error handling feat!: config management and error handling Aug 18, 2022
@mabdh mabdh marked this pull request as ready for review August 18, 2022 04:34
@mabdh mabdh requested a review from ravisuhag August 18, 2022 07:15
@mabdh mabdh requested review from rohilsurana and spy16 August 18, 2022 08:23
@ravisuhag
Copy link
Member

@mabdh Let's also move the shield sample config file to the config package itself and call it config.yaml Ideally, we would want to encourage the behavior of using shield server init to generate server config and not manually copy-paste the file.

@mabdh mabdh requested a review from ravisuhag August 19, 2022 04:48
@ravisuhag ravisuhag merged commit 50883e9 into main Aug 19, 2022
@ravisuhag ravisuhag deleted the cli-config-improvement branch August 19, 2022 05:17
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.

Shield cli config management and error handling

5 participants