added app upload health checks #153
Conversation
|
Please don't merge yet. I'm testing the openAPI app creation will push in the same PR. |
|
This PR is complete and tested and ready to for a review. |
| // replace edaa73d40238ee60874a853dc3ccaa6f | ||
| // with id from above and bunch of other data to | ||
| // not get same app id when verified | ||
| newOpenapiString := strings.Replace(openapiString, `"edaa73d40238ee60874a853dc3ccaa6f"`, `"`+id+`"`, 1) |
There was a problem hiding this comment.
What are we doing here? 😆
Can't we instead just change the actual struct? and marshal/unmarshal?
This will somehow break - I just don't know how lol
|
|
||
| // TODO: More testing for onprem health checks | ||
| if project.Environment == "cloud" { | ||
| appHealthChannel := make(chan AppHealth) |
There was a problem hiding this comment.
If you call the other one "pythonAppHealth", please call this "openapiAppHealth" or something to make it clear. I wasn't able to differ them until I read deeper into them.
| ExecutionID: "", | ||
| } | ||
|
|
||
| appZipUrl := "https://github.com/yashsinghcodes/python-apps/raw/refs/heads/master/shuffle-tools-copy.zip" |
There was a problem hiding this comment.
If this is going to prod, we need it in a shuffle repo (:
| }() | ||
|
|
||
| baseUrl := "https://shuffler.io" | ||
| if os.Getenv("SHUFFLE_CLOUDRUN_URL") != "" { |
There was a problem hiding this comment.
Again, same as the other place:
BASE_URL > CLOUDRUN_URL. Cloudrun is used to OVERRIDE the BASE_URL. That is it's whole purpose.
| baseUrl = os.Getenv("SHUFFLE_CLOUDRUN_URL") | ||
| } | ||
|
|
||
| if project.Environment == "onprem" { |
There was a problem hiding this comment.
Check for not cloud instead of for onprem.
| } | ||
|
|
||
| if res.StatusCode != 200{ | ||
| log.Printf("[ERROR] Failed to upload an ops app. Response: %s", string(response)) |
There was a problem hiding this comment.
There may be data here that is useful such as logs. How do you handle those cases so that you know what to do?
| appHealth.AppId = appData.Id | ||
|
|
||
| // wait 5 second before execution | ||
| time.Sleep(5 * time.Second) |
|
|
||
| appExecuteData, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| log.Printf("[ERROR] Failed to read app execution data") |
|
|
||
| appHealth.Run = true | ||
| appHealth.ExecutionID = executionData.Id | ||
| for executionData.Result == "" { |
There was a problem hiding this comment.
Avoid infinite loops. Add indexing with a max limit if you are gonna do it this way.
While loops can go die in a fire.
|
|
||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != 200 { |
There was a problem hiding this comment.
Same here; maybe resp body could be useful?
No description provided.