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

Add download and path urls #174

Merged
merged 2 commits into from
Dec 3, 2019
Merged

Add download and path urls #174

merged 2 commits into from
Dec 3, 2019

Conversation

ruflin
Copy link
Member

@ruflin ruflin commented Dec 2, 2019

At the moment, only the /search endpoint contains the download url. This is now also added to the package info. In addition a field path is added which is the path to the package info. This should help with breaking changes in the future so EPM can rely on this information to find the info about a package instead of having to hard code it.

Copy link

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

I like the flexibility/durability this adds. My only reservation is that path feel vague or generic, but I don't have a better name in mind at the moment, so let's take the incremental win now.

To expand a bit, path feels more like a type like string or URL than a descriptive name. e.g. We used download instead of download_path but both give the reader a sense of the purpose. Especially when you see "/package/example-1.0.0.tar.gz". But with path I ask "path to where?". What's at "/package/example-1.0.0"? I think that's the package info endpoint, but info seems as vague a key as path and I think I'd expect an object there; not a path/string.

Perhaps we could put the URLs under their own key like:

"paths": {
  "download": "/package/example-1.0.0.tar.gz",
  "info": "/package/example-1.0.0",
}

One nice thing about an object is we could nest the items that were more specific to how the registry works vs unique information about the package.

Again, I don't see an obvious replacement and am happy to keep things moving. Just want to put what I'm thinking in writing for us to consider.

At the moment, only the `/search` endpoint contains the download url. This is now also added to the package info. In addition a field `path` is added which is the path to the package info. This should help with breaking changes in the future so EPM can rely on this information to find the info about a package instead of having to hard code it.
@ruflin
Copy link
Member Author

ruflin commented Dec 3, 2019

@jfsiii I share the exact same sentiment around naming and potentially having a better structure. Putting it in an object could be helpful, need to think more about this in the context of all fields.

For now will merge it and I kind of hope that if we add more fields it will become clear where things belong.

@ruflin ruflin merged commit 490ed41 into elastic:master Dec 3, 2019
@ruflin ruflin deleted the download-path branch December 3, 2019 13:42
@ruflin ruflin self-assigned this Dec 5, 2019
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.

2 participants