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.

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

feat(resource-link): Add Service Resource commands #800

Merged
merged 13 commits into from
Feb 22, 2023

Conversation

awilliams-fastly
Copy link
Collaborator

@awilliams-fastly awilliams-fastly commented Feb 7, 2023

https://developer.fastly.com/reference/api/services/resource/

This adds a resource-link sub-command which has associated create, delete, list, and update commands.

USAGE
  fastly resource-link <command> [<args> ...]

Manipulate Fastly service resource links

COMMANDS
  resource-link
  create    Create a Fastly service resource link
  delete    Delete a resource link for a Fastly service version
  describe  Show detailed information about a Fastly service resource link
  list      List all resource links for a Fastly service version
  update    Update a resource link for a Fastly service version

SEE ALSO
  https://developer.fastly.com/reference/cli/resource-link/

Upgraded github.com/fastly/go-fastly/v7 v7.1.0 => v7.2.0

Some of the names in the API may be confusing. I've done my best to add clarification whenever possible. Specifically:

  • A resource, as named by the API, is truly a link between a service and a resource (e.g. an Object Store), not the resource itself. I've tried to use "resource link" wherever appropriate, as also used by the API docs:

    A resource represents a link between a resource type and a service version.

    Update 1d2eb20: uses a different base command name resource-link.

  • In some places, the API mixes id and resource_id which both refer to the ID of the resource link, and not the resource itself. I've changed this so that id always refers the resource link, and resource_id to the linked to object. Fixed by resource: Improve parameter naming; support JSON encode go-fastly#394

  • It's possible to provide a resource link name. If set, this is an alias for the resource. Otherwise, whatever name was used when creating the resource itself will be the name to reference from the service for the resource. I've added to the description that this is an alias. See this code comment: https://github.com/fastly/go-fastly/blob/ef4694a59779f202d8ea630b41333eb2b76969e8/fastly/resource.go#L86-L88

Keeping this as a draft until a new go-fastly release is created that incorporates fastly/go-fastly#394.

https://developer.fastly.com/reference/api/services/resource/

This adds a `service-resource` sub-command which has associated
create, delete, list, and update commands.

Some of the names in the API may be confusing.
I've done my best to add clarification whenever possible.
Specifically:

- A resource, as named by the API, is truly a _link_ between a service
  and a resource (e.g. an Object Store), not the resource itself.
  I've tried to use "resource **link**" wherever appropriate, as also
  used by the API docs:
   > A resource represents a link between a resource type and a service version.

- In some places, the API mixes `id` and `resource_id` which both
  refer to the ID of the resource _link_, and not the resource itself.
  I've changed this so that `id` always refers the resource link,
  and `resource_id` to the linked to object.
  See this code comment: https://github.com/fastly/go-fastly/blob/ef4694a59779f202d8ea630b41333eb2b76969e8/fastly/resource.go#L125-L129

- It's possible to provide a resource link name.
  If set, this is an alias for the resource.
  Otherwise, whatever name was used when creating the resource itself
  will be the name to reference from the service for the resource.
  I've added to the description that this is an _alias_.
  See this code comment: https://github.com/fastly/go-fastly/blob/ef4694a59779f202d8ea630b41333eb2b76969e8/fastly/resource.go#L86-L88

I'd like to look into modifying `go-fastly` to remove the need for the `jsonResource` type, and also into updating the API to be more consistent with `id` vs `resource_id`.
This comes with changes specific to the Resource API:
 - Renamed `resource_id` to `id` when referencing resource (link) ID
 - Added `json` tags to the Resource type
@joeshaw
Copy link
Member

joeshaw commented Feb 8, 2023

Bikeshedding on the command name, I wonder if service-link or resource-link are clearer? The resource itself (object store, secret store, etc.) is already created. This is linking it to a particular service version.

@Integralist
Copy link
Collaborator

@joeshaw to be honest for me either the current service-resource or your service-link works in my mind, and that's because it all depends on how I think about the command 'in my head' at the time of using the command 😬

COMMAND MY INTERNAL UNDERSTANDING WHEN SEEING THE COMMAND
service-resource I have a resource I want to attach to my service
service-link I want to link something up to my service

I'm not sure which 'description' I'm more likely to say in my head when looking at the command.

At a push my gut says service-link but then I recall an internal document from Patrick saying why "link" was a bad name and that's why they changed from "Links API" to "Resource API".

@awilliams-fastly
Copy link
Collaborator Author

The UI will eventually be updated with a top-level Resources tab. That page will contain a list of resources (e.g. Object Stores), with a Link to services button alongside each resource.

For this reason, I think that resource-link may best align with the UI's verbiage.

This better matches the language used in Fastly's web UI.
@awilliams-fastly awilliams-fastly marked this pull request as ready for review February 13, 2023 17:05
pkg/commands/resourcelink/serviceresource_test.go Outdated Show resolved Hide resolved
pkg/commands/resourcelink/create.go Outdated Show resolved Hide resolved
pkg/commands/resourcelink/json.go Outdated Show resolved Hide resolved
@Integralist
Copy link
Collaborator

CI is failing with some weird error 🤔

Screenshot 2023-02-17 at 09 58 33

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.

LGTM 👍🏻

@awilliams-fastly awilliams-fastly changed the title feat(service-resource): Add Service Resource commands feat(resource-link): Add Service Resource commands Feb 21, 2023
@awilliams-fastly
Copy link
Collaborator Author

CI is failing with some weird error 🤔

Screenshot 2023-02-17 at 09 58 33

Not sure what that was from. But a re-run of the failing CI job passed, and staticcheck also passes for me locally.

@awilliams-fastly awilliams-fastly merged commit 69dbc10 into main Feb 22, 2023
@awilliams-fastly awilliams-fastly deleted the awilliams/resource-api branch February 22, 2023 03:08
@Integralist Integralist added the enhancement New feature or request label Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants