-
Notifications
You must be signed in to change notification settings - Fork 0
send JSON object in body instead of uploading edited recipe #174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,8 @@ | ||
| import { useState } from "react"; | ||
| import { v4 as uuidv4 } from "uuid"; | ||
| import { Layout, Typography } from "antd"; | ||
| import { getJobStatus, addRecipe } from "./utils/firebase"; | ||
| import { getFirebaseRecipe, jsonToString } from "./utils/recipeLoader"; | ||
| import { getJobStatus, updateJobStatusTimestamp } from "./utils/firebase"; | ||
| import { getFirebaseRecipe, recipeToString } from "./utils/recipeLoader"; | ||
| import { getSubmitPackingUrl, JOB_STATUS } from "./constants/aws"; | ||
| import { FIRESTORE_FIELDS } from "./constants/firebase"; | ||
| import { SIMULARIUM_VIEWER_URL } from "./constants/urls"; | ||
| import { | ||
| useCurrentRecipeData, | ||
|
|
@@ -47,67 +45,41 @@ function App() { | |
| recipeString: string | ||
| ): Promise<boolean> => { | ||
| const originalRecipe = await getFirebaseRecipe(recipeId); | ||
| return !(jsonToString(originalRecipe) == recipeString); | ||
| }; | ||
|
|
||
| const recipeToFirebase = ( | ||
| recipe: string, | ||
| path: string, | ||
| id: string | ||
| ): object => { | ||
| const recipeJson = JSON.parse(recipe); | ||
| if (recipeJson.bounding_box) { | ||
| const flattened_array = Object.assign({}, recipeJson.bounding_box); | ||
| recipeJson.bounding_box = flattened_array; | ||
| } | ||
| recipeJson[FIRESTORE_FIELDS.RECIPE_PATH] = path; | ||
| recipeJson[FIRESTORE_FIELDS.NAME] = id; | ||
| return recipeJson; | ||
| return !(recipeToString(originalRecipe) == recipeString); | ||
| }; | ||
|
|
||
| const submitRecipe = async ( | ||
| recipeId: string, | ||
| configId: string, | ||
| recipeString: string | ||
| ) => { | ||
| let firebaseRecipe = "firebase:recipes/" + recipeId; | ||
| const firebaseConfig = configId | ||
| ? "firebase:configs/" + configId | ||
| : undefined; | ||
| const recipeChanged: boolean = await recipeHasChanged( | ||
| recipeId, | ||
| recipeString | ||
| ); | ||
| if (recipeChanged) { | ||
| const recipeId = uuidv4(); | ||
| firebaseRecipe = "firebase:recipes_edited/" + recipeId; | ||
| const recipeJson = recipeToFirebase( | ||
| recipeString, | ||
| firebaseRecipe, | ||
| recipeId | ||
| ); | ||
| try { | ||
| await addRecipe(recipeId, recipeJson); | ||
| } catch (e) { | ||
| setJobStatus(JOB_STATUS.FAILED); | ||
| setJobLogs(String(e)); | ||
| return; | ||
| } | ||
| } | ||
| const firebaseRecipe = recipeChanged | ||
| ? undefined | ||
| : "firebase:recipes/" + recipeId; | ||
| const firebaseConfig = configId | ||
| ? "firebase:configs/" + configId | ||
| : undefined; | ||
|
|
||
| const url = getSubmitPackingUrl(firebaseRecipe, firebaseConfig); | ||
| const request: RequestInfo = new Request(url, { method: "POST" }); | ||
| const requestBody = recipeChanged ? recipeString : undefined; | ||
| const request: RequestInfo = new Request(url, { method: "POST", body: requestBody }); | ||
|
Comment on lines
+68
to
+69
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Content-Type: application/json? |
||
| start = Date.now(); | ||
| const response = await fetch(request); | ||
| setJobStatus(JOB_STATUS.SUBMITTED); | ||
| setJobLogs(""); | ||
| const data = await response.json(); | ||
| if (response.ok) { | ||
| const data = await response.json(); | ||
| setJobId(data.jobId); | ||
| setJobStatus(JOB_STATUS.STARTING); | ||
| return data.jobId; | ||
| } else { | ||
| const errorText = await response.text(); | ||
| setJobStatus(JOB_STATUS.FAILED); | ||
| setJobLogs(JSON.stringify(data)); | ||
| setJobLogs(errorText); | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -124,6 +96,9 @@ function App() { | |
| const checkStatus = async (jobIdFromSubmit: string) => { | ||
| const id = jobIdFromSubmit || jobId; | ||
| let localJobStatus = await getJobStatus(id); | ||
| if (localJobStatus) { | ||
| setJobStatus(localJobStatus.status); | ||
| } | ||
| while ( | ||
| localJobStatus?.status !== JOB_STATUS.DONE && | ||
| localJobStatus?.status !== JOB_STATUS.FAILED | ||
|
|
@@ -138,6 +113,11 @@ function App() { | |
| setJobStatus(newJobStatus.status); | ||
| } | ||
| } | ||
|
|
||
| // Update the job status timestamp after reading the final status to | ||
| // ensure we have the most recent timestamp for retention policy | ||
| await updateJobStatusTimestamp(id); | ||
ascibisz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| const range = (Date.now() - start) / 1000; | ||
| if (localJobStatus.status == JOB_STATUS.DONE) { | ||
| setPackingResults({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ const InputSwitch = (props: InputSwitchProps): JSX.Element => { | |
| } | ||
| if (typeof value == "number") { | ||
| value = value * conversion; | ||
| value = Number(value.toFixed(4)); | ||
| value = Number(value.toFixed(2)); | ||
ascibisz marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, we were keeping 4 digits after the decimal point and it was getting cut off in the box on the form. It feels like 2 digits after the decimal point is sufficient? |
||
| } | ||
| return value; | ||
| }, [getCurrentValue, id, min, conversion, dataType]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,12 +5,16 @@ const SUBMIT_PACKING_ECS = "https://bda21vau5c.execute-api.us-west-2.amazonaws.c | |
| const S3_BASE_URL = "https://s3.us-west-2.amazonaws.com"; | ||
|
|
||
| export const getSubmitPackingUrl = ( | ||
| recipe: string, | ||
| recipe?: string, | ||
| config?: string, | ||
| ) => { | ||
| let url = `${SUBMIT_PACKING_ECS}?recipe=${recipe}`; | ||
| if (config) { | ||
| url += `&config=${config}`; | ||
| let url = SUBMIT_PACKING_ECS; | ||
| if (recipe && config) { | ||
| url += `?recipe=${recipe}&config=${config}`; | ||
| } else if (recipe) { | ||
| url += `?recipe=${recipe}`; | ||
| } else if (config) { | ||
| url += `?config=${config}`; | ||
|
Comment on lines
+11
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Safer to encode with this API as long as the result works on the backend: https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams |
||
| } | ||
| return url; | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ export const FIRESTORE_FIELDS = { | |
| export const RETENTION_POLICY = { | ||
| RETENTION_PERIODS: { | ||
| RECIPES_EDITED: 24 * 60 * 60 * 1000, // 24 hours | ||
| JOB_STATUS: 24 * 60 * 60 * 1000, // 24 hours | ||
| JOB_STATUS: 30 * 24 * 60 * 60 * 1000, // 30 days | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since we're re-using results from previous job runs now using the job_status collection, we want to increase it's retention policy |
||
| }, | ||
|
|
||
| TIMESTAMP_FIELD: "timestamp", | ||
|
|
||
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.
double check if this is still necessary