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. |
health.go
Outdated
| // 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
health.go
Outdated
|
|
||
| // 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.
health.go
Outdated
| 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.
There was a problem hiding this comment.
I think this is what you meant, reading BASE_URL first and if there is CLOUDRUN_URL we just override it?
health.go
Outdated
| 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?
There was a problem hiding this comment.
I'm logging the response, with string(response)
| appHealth.AppId = appData.Id | ||
|
|
||
| // wait 5 second before execution | ||
| time.Sleep(5 * time.Second) |
There was a problem hiding this comment.
This is needed for the cloud run function to be deployed.
|
|
||
| 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?
|
I see no changes here for 2 weeks. What do? |
Bump. Please pay attention to your notifications (: |
|
Oops, I forgot to make the changes. I'll be pushing them by the end of the day. |
Did it get pushed by EOD? (: |
|
@yashsinghcodes update? D: |
|
Hey @frikky looks like you missed the above commits :) |
No description provided.