Bot Download API #26
itzaname
Reference in New Issue
Block a user
Delete Branch "bot-dl"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Please tell me if there is a different way I should be doing this.
@@ -44,0 +46,4 @@Url string `json:"url"`} // @name BotDownloadUrltype FileInfo struct {DTO is for external interfaces not calling internal services. So basically what the API would receive from a client or respond with to an API client.
FileInfois for an internal service call and does not belong here. I'd honestly just inline the struct like:@@ -27,3 +32,4 @@}return &TimesHandler{Handler: baseHandler,client: http_client,Using snake_case while surrounded by camelCase is a war crime.
@@ -320,0 +334,4 @@// @Success 200 {object} dto.Response[dto.BotDownloadUrl]// @Failure 404 {object} dto.Error "Time not found"// @Failure 404 {object} dto.Error "Time does not have a Bot"// @Failure 404 {object} dto.Error "Bot not found"You've got 3 404s but the only one will be used as you can see in the generated docs.
@@ -320,0 +336,4 @@// @Failure 404 {object} dto.Error "Time does not have a Bot"// @Failure 404 {object} dto.Error "Bot not found"// @Failure default {object} dto.Error "General error response"// @Router /time/{id}/download-url [get]This is an RPC like action not a RESTful url. Do something like this:
@@ -320,0 +379,4 @@ctx.JSON(statusCode, dto.Error{Error: errorMessage,})log.WithError(err).Error(You're using
WithErrorbut no error was actually set.@@ -320,0 +410,4 @@// fetch download url from storage service// Build the full URL.fullURL := h.url + botData.FileIDUse
url.JoinPathas it will handle adding/as needed.@@ -320,0 +451,4 @@ctx.JSON(statusCode, dto.Error{Error: errorMessage,})log.WithError(err).Error(Another error log when err would be nil.
@@ -320,0 +473,4 @@}// Return the download urlctx.JSON(http.StatusOK, dto.Response[dto.BotDownloadUrl]{Simplify things and just 302 to the download url instead.
@@ -60,0 +67,4 @@}}// WithStorageUrl sets the data gRPC clientThis aint no grpc client
@@ -76,3 +93,3 @@// Times handlertimesHandler, err := handlers.NewTimesHandler(handlerOptions...)timesHandler, err := handlers.NewTimesHandler(cfg.httpClient, cfg.storageUrl, handlerOptions...)You added
WithStorageUrlandWithHttpClientfor the api options but then ignore that pattern when working with the times handler and slapped some new args in front...The Handler type and HandlerOption is not general enough, I don't understand how to implement the pattern here. Adding httpClient and storageUrl to Handler would pollute the other handlers with irrelevant and unused fields.
Just add it to the handler it's already getting thrown into
RouterConfigand the cli I'm not going to lose sleep overHandler.@@ -53,3 +64,4 @@api.WithPort(ctx.Int("port")),api.WithDevClient(devConn),api.WithDataClient(dataConn),api.WithHttpClient(&http.Client{}),Set a timeout when making the client
@@ -50,1 +58,4 @@// Storage service http clientstorageUrl := ctx.String("storage-host")httpClient := http.Client{Honestly looking at this more I'm not sure why this http client struct is being pre-created instead of just being created in the one function that uses it.
@@ -51,0 +59,4 @@// Storage service http clientstorageUrl := ctx.String("storage-host")httpClient := http.Client{Timeout: 10,This is 10 nano seconds
WIP: Bot Download APIto Bot Download API@@ -76,3 +86,3 @@// Times handlertimesHandler, err := handlers.NewTimesHandler(handlerOptions...)timesHandler, err := handlers.NewTimesHandler([]handlers.HandlerOption{We got there eventually
@@ -318,2 +323,4 @@})}// @Summary Get bot download url by time IDActually, probably need to update this since you're not really getting the dl url.
Generate the api docs again and you're good.
Please merge and get staging working, otherwise deploy directly to prod. Reminder that it also needs a new permission category "Storage.Read" on the dev portal.
Is "Storage" too generic? Bots.Read?
The only thing it could be confused with is "Datastore" for playerdata & equipment. Bots are the only thing on storage, but I think being precise is a good idea. I'll change it to Bots. We can change it to Assets or something if we end up hosting maps there too and want them on the same category.
@@ -123,2 +136,4 @@v1.GET("/rank", rankHandler.List)}v1_bots := r.Group("/api/v1")Missed this one sneaky snake_case. I'll note I don't know if duplicate groups can work like this but we will find out.
It's live https://api.staging.strafes.net/docs/index.html