Implement datasets data model #137
Conversation
andresgutgon
force-pushed
the
feature/datasets-data-model
branch
3 times, most recently
from
adb6771 to
bc5aee9
Compare
| }, | ||
| db = database, | ||
| ) { | ||
| const deleteResult = await disk.delete(dataset.fileKey) |
There was a problem hiding this comment.
This deletes the file in disk (Filesystem or S3 or other clouds). Do you think we should do in another way, or is blocking the request fine?
There was a problem hiding this comment.
fine for now although it would have been more scalable to delete the file in a job afterwards
There was a problem hiding this comment.
Let me try in production with S3 enabled and we can move it to a job
andresgutgon
force-pushed
the
feature/datasets-data-model
branch
from
8c0f56c to
ee4887c
Compare
| description, | ||
| submitStr, | ||
| model, | ||
| }: Props<TServerAction>) { |
There was a problem hiding this comment.
Improve generic for destroy modal
| 'application/vnd.ms-excel', | ||
| 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', | ||
| 'application/vnd.oasis.opendocument.spreadsheet', | ||
| ] | ||
| const MAX_SIZE = 3 |
There was a problem hiding this comment.
3mb only? that's very small, i'd push it to 25 or so
There was a problem hiding this comment.
well the theory is that they want to evaluate on a large body of data no?, 3mb doesn't push our infra hard at all and it can be limiting for no reason
There was a problem hiding this comment.
25 then. But node server will handle it?
We'll have to do this https://nextjs.org/docs/app/api-reference/next-config-js/serverActions#bodysizelimit
Although the best approach would be to direct upload to S3 and then process the file when S3 responds. But we need to change how upload is done
There was a problem hiding this comment.
I'll start with 15MB and see if someone complains
| return { | ||
| data, | ||
| mutate, | ||
| createFormAction, |
There was a problem hiding this comment.
you shouldn't use this as it won't be compatible with the json-based server action
There was a problem hiding this comment.
I need this to submit a multi-part form with attachment. This is already working
| @@ -22,15 +24,47 @@ export const createDataset = async ( | |||
| } | |||
| disk: DiskWrapper | |||
There was a problem hiding this comment.
this is called disk but it can be other file storage no? like s3 or others
There was a problem hiding this comment.
This is how they called in flydrive
andresgutgon
force-pushed
the
feature/datasets-data-model
branch
from
ee4887c to
c026196
Compare
| --> statement-breakpoint | ||
| CREATE INDEX IF NOT EXISTS "datasets_workspace_idx" ON "latitude"."datasets" USING btree ("workspace_id");--> statement-breakpoint | ||
| CREATE INDEX IF NOT EXISTS "datasets_author_idx" ON "latitude"."datasets" USING btree ("author_id");--> statement-breakpoint | ||
| CREATE UNIQUE INDEX IF NOT EXISTS "datasets_workspace_id_name_index" ON "latitude"."datasets" USING btree ("workspace_id","name"); |
There was a problem hiding this comment.
☝️ Added 3 indexes
- Workspace
- Author
- Workspace + Name
I'll add UI validation for workspace + name uniqueness. This was a recommendation from @csansoon . He said it's nice to don't repeat names in datasets and I agree.
What?
☝️ Table for storing data related with uploaded datasets
What?
workspace_idandauthor_id.datasets.namein combination withworkspace_id[',', '\t', ' ', ';']. We need to know this in order to read the CSV. Nothing is easy 😂Next TODO