-
Notifications
You must be signed in to change notification settings - Fork 46
Add script for copying prod run #242
Add script for copying prod run #242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the indentation thing is a blocker, but I just figured I'd call it out so you could take a peek: https://github.com/lukebjerring/wptdashboard/blob/56816891b33d1bab95a8928495e7ec4f550bf0b7/test_run_handler.go#L39-L56
test_run_handler.go
Outdated
|
||
// Fetch pre-uploaded Token entity. | ||
suppliedSecret := r.URL.Query().Get("secret") | ||
var queryParams url.Values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Strange indentation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed - I'm not sure which editor is doing that, I bounce between pycharm/gogland/sublime/atom
Reverted the secret-adding part, as this is now covered by a python script in #248 which is safe. |
test_run_handler.go
Outdated
@@ -67,16 +74,19 @@ func handlePost(w http.ResponseWriter, r *http.Request) { | |||
} | |||
testRun.CreatedAt = time.Now() | |||
|
|||
fmt.Fprintf(w, "got... %v", testRun) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this output is (was) a bit debuggy. The output is now just the JSON blob as stored in the DB, which is a pretty standard behaviour in a REST API.
* Migrate populate-dev-data to a Python script. * Add script, Use CamelCase property names * Fix capitalization * Add todo for reusing #242 code * Re-use add_production_run from populate_dev_data * Changes as per review * Include port-forwarding in docker * Remove whitespace
Note that this should not be merged until #241 is resolved (as it includes a local upload-token setup hack to get this to work, for completeness).