refactor: daily now uses the fact api can take multiple packages #3
Conversation
…turn json api docs noted that you can provide a comma separated list of packages and it'll return the array
|
some tests: regular: with json: nonsense package names just return 0 downloads, rather than an error, so "error checking" for invalid packages isn't really a thing. r-hub/cranlogs.app#18 I also filed: r-hub/cranlogs.app#41 which came up when I originally removed a "/" to do a gut check on the status code properly erroring (it didn't) then was getting mysterious marshalling errors instead |
|
kk the 404 is now added in: r-hub/cranlogs.app#42 |
| @@ -18,31 +19,36 @@ type DailyDownload struct { | |||
| Downloads int `json:"downloads"` | |||
| } | |||
|
|
|||
| func GetDaily(period, pkg string) (Daily, error) { | |||
| func GetDaily(period Period, pkgs []string) ([]Daily, error) { | |||
| // weirdly the API returns an array of length 1 | |||
There was a problem hiding this comment.
this is because of that comma separated packages - keeps the same data structure if 1 package or many..... note: need to update to remove the comment now
There was a problem hiding this comment.
I did not know we could pass multiple packages
| // https://github.com/r-hub/cranlogs.app/issues/41 is resolved but at least | ||
| // its there | ||
| if resp.StatusCode != 200 { | ||
| return daily, err |
There was a problem hiding this comment.
this is.... bare minimum ok - ideally would consider more about what alternative responses could come in then handle them. One huge issue is essentially this will currently swallow the result that isn't an error. So for example, the boy json with the error message currently is being swallowed - so this needs to get refactored to likely generate a better, processed error.
| } | ||
|
|
||
| err = json.Unmarshal(body, &daily) | ||
|
|
||
| // if this errors given the current 200 status code returning an error response |
There was a problem hiding this comment.
this is now (partially) fixed due to the new deployment that does 404 (at least for some cases)
This refactors daily to take advantage of don't need to make multiple requests as its valid to include a comma separated package list
This is also why (to your question/comment in the code) why even for a single package it returns an array, to keep the return structure consistent whether single or multiple packages (i presume at least)
In addition, I added the conditional capability to return json - this would make it easier to interact with this in programmatic settings/compose with other tools