Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(config-store): Add Config Store commands #829

Merged
merged 4 commits into from Mar 16, 2023
Merged

feat(config-store): Add Config Store commands #829

merged 4 commits into from Mar 16, 2023

Conversation

awilliams-fastly
Copy link
Collaborator

@awilliams-fastly awilliams-fastly commented Feb 16, 2023 •
edited

Related go-fastly change: fastly/go-fastly#398

USAGE
  fastly config-store <command> [<args> ...]

COMMANDS
  config-store
  create         Create a new config store
  delete         Delete a config store
  describe       Retrieve a single config store
  list           List config stores
  list-services  List config store's services
  update         Update a config store

USAGE
  fastly config-store-entry <command> [<args> ...]

COMMANDS
  config-store-entry
  create    Create a new config store item
  delete    Delete a config store item
  describe  Retrieve a single config store item
  list      List config store items
  update    Update a config store item

@awilliams-fastly awilliams-fastly mentioned this pull request Feb 16, 2023
Merged
awilliams-fastly added a commit to fastly/go-fastly that referenced this pull request Mar 2, 2023
@awilliams-fastly
Add Config Store and Config Store Item methods
92a6ab1 
- https://developer.fastly.com/reference/api/services/resources/config-store/
- https://developer.fastly.com/reference/api/services/resources/config-store-item/

Related `cli` change: fastly/cli#829
@awilliams-fastly awilliams-fastly marked this pull request as ready for review March 2, 2023 19:15
awilliams-fastly added a commit to fastly/go-fastly that referenced this pull request Mar 9, 2023
@awilliams-fastly
feat: Add Config Store support (#398)
0c20056 
- https://developer.fastly.com/reference/api/services/resources/config-store/
- https://developer.fastly.com/reference/api/services/resources/config-store-item/

Related `cli` change: fastly/cli#829
@Integralist Integralist marked this pull request as draft March 14, 2023 10:10
@Integralist
Copy link
Collaborator

👋🏻 @awilliams-fastly

I've converted this PR to draft for the moment as the CI has been broken for a while but I'm being pinged daily to review (and I believe this PR is still being worked on).

Just let me know when you're ready for me to review. Thanks.

@awilliams-fastly
Copy link
Collaborator Author

awilliams-fastly commented Mar 14, 2023 •
edited

@Integralist sorry about that, I should have left it as draft.

We'll need to bump the version of go-fastly once a release is made that includes the config store additions in order for the tests to pass.

@awilliams-fastly
feat(config-store): Add Config Store commands
f429d79 
- https://developer.fastly.com/reference/api/services/resources/config-store/
- https://developer.fastly.com/reference/api/services/resources/config-store-item/

Related `go-fastly` change: fastly/go-fastly#398

```
USAGE
  fastly config-store <command> [<args> ...]

COMMANDS
  config-store
  create         Create a new config store
  delete         Delete a config store
  describe       Retrieve a single config store
  list           List config stores
  list-services  List config store's services
  update         Update a config store
```

```
USAGE
  fastly config-store-entry <command> [<args> ...]

COMMANDS
  config-store-entry
  create    Create a new config store item
  delete    Delete a config store item
  describe  Retrieve a single config store item
  list      List config store items
  update    Update a config store item
```
@awilliams-fastly
Fix unrelated failing 'backend' test
e846534 
Result of this change to `go-fastly`:
fastly/go-fastly#405
@awilliams-fastly awilliams-fastly marked this pull request as ready for review March 15, 2023 16:33
@Integralist Integralist added the enhancement New feature or request label Mar 16, 2023
Integralist
Integralist requested changes Mar 16, 2023
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Thanks @awilliams-fastly for this PR. It looks great!

I just have a few comments related to code-base consistency and conventions.

pkg/commands/configstore/describe.go Outdated Show resolved Hide resolved
pkg/commands/configstore/describe.go Outdated Show resolved Hide resolved
pkg/commands/configstore/describe.go Outdated Show resolved Hide resolved
pkg/commands/configstore/configstore_test.go Outdated Show resolved Hide resolved
pkg/commands/configstore/configstore_test.go Outdated Show resolved Hide resolved
pkg/commands/configstoreentry/update.go Show resolved Hide resolved
pkg/commands/configstore/list.go Outdated Show resolved Hide resolved
pkg/commands/configstore/list_services.go Outdated Show resolved Hide resolved
pkg/commands/configstore/update.go Outdated Show resolved Hide resolved
pkg/commands/configstoreentry/list.go Outdated Show resolved Hide resolved
@awilliams-fastly
Address PR feedback
ad3ce1c 
- Use testutil.TestScenario instead of custom struct
- Drop testing of whether API methods were invoked or not
- Alphabetical ordering of command line flags
- Remove unused 'prefix' argument from text Print function
@awilliams-fastly
Simplify text.PrintConfigStore
bab0af6 
This removes the previous complexity of using an interface for the
config store and the optional metadata. Instead, just use two arguments.
Integralist
Integralist approved these changes Mar 16, 2023
View reviewed changes
Copy link
Collaborator

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Awesome. Thank you @awilliams-fastly

@awilliams-fastly awilliams-fastly merged commit 183f127 into main Mar 16, 2023
6 checks passed
@awilliams-fastly awilliams-fastly deleted the awilliams/config-store branch March 16, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Integralist Integralist Integralist approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@awilliams-fastly @Integralist