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

Feature Request: Support passing service_name in place of service_id #188

Closed
benzvan opened this issue Jan 8, 2021 · 8 comments
Closed
Labels
enhancement New feature or request

Comments

@benzvan
Copy link
Contributor

benzvan commented Jan 8, 2021

Service names are human understandable and service IDs are random hashes. Since the CLI already supports searching for service IDs from a service name, a user could be allowed to use a --service_name flag and let the CLI fetch the service ID from the fastly API.

This is a relatively large change in the number of files touched since almost every function takes --service_id as a flag but it could be just a few lines per file to call out to the service search function for the value.

@benzvan
Copy link
Contributor Author

benzvan commented Jan 8, 2021

Just a small note: I had started to work on this in my enhancement to support edge dictionaries but it didn't make sense to combine two new features in a single merge.

@Integralist Integralist added the enhancement New feature or request label Jan 8, 2021
@Integralist
Copy link
Collaborator

Hi @benzvan thanks for opening this issue!

If done right this could be a nice addition, as far as user experience is concerned (in the sense that people are more likely to remember the name of their service than a random ID).

That said I'm not sure what the performance implications are and how that will affect the user experience. For every single command they execute, they would have to wait first for an API request/response to complete before we could then execute their command.

One mitigation to that problem would be to cache the service name lookup (e.g. memoization).

Let me know your thoughts.

Thanks Ben.

@benzvan
Copy link
Contributor Author

benzvan commented Jan 8, 2021

While the response from fastly is usually pretty fast, I like the idea of memoizing it, assuming the mapping would be consistent in the long term and the data storage was not intrusive. I don't know if golang has any built-in functions that would allow temporary storage between calls and accommodate multiple OSes, but I assume this wouldn't be the first project to need it. Many apps already store data in hidden directories in ~, so it probably isn't a big deal. Setting environment variables, which I believe the cli already reads, could also be valid.

@benzvan
Copy link
Contributor Author

benzvan commented Jan 8, 2021

Actually...thinking about it...just enhancing the way the current service ID list/search works to store the response would be effective.

@Integralist
Copy link
Collaborator

I'm open to the idea, as long as we could work towards an efficient solution that wasn't too intrusive. I'll leave it with you to have a think about. Let me know what approach you finalise on and we can discuss if it still makes sense to implement before you go ahead with a PR. Thanks Ben.

@benzvan
Copy link
Contributor Author

benzvan commented Jan 11, 2021

I took a look at the code for some ideas on how to make this fit well.

I think we could do it in a couple steps:

  1. Create a memo file using a similar pattern to the config data file, storing yaml, json, or a binary object in the UserConfigDir https://github.com/fastly/cli/blob/master/pkg/config/data.go#L100-L109
    Ideally, this would use the existing go memoization library but write the data to a file to use between cli executions. https://godoc.org/golang.org/x/tools/internal/memoize
  2. Work it out in a smaller way with one module that has a similar issue, edgedictionaryitem, which requires the user to translate dictionary names to dictionary IDs.
  3. Come back to this issue for service ID using the same pattern.

@Integralist
Copy link
Collaborator

Integralist commented Jan 14, 2021

I like the idea of solving this problem on a smaller scale, but with a reusable pattern that will easily translate to the original issue of dealing with Service IDs. If you're happy to take a crack at this for edge dictionary, then that should give us a quicker feedback loop on the feasibility of it without (hopefully) sinking too much time into it if it doesn't work out.

Thanks Ben!

@Integralist
Copy link
Collaborator

Closed by #495

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

No branches or pull requests

2 participants