feat: list uploads joins to backup table to get backup urls #2352
Conversation
gobengo
changed the title
There was a problem hiding this comment.
Could we get more info about why we are adding this table? We actually had it before and moved towards having a column within upload table later on.
Also a point in migration that you may not know, we have a folder https://github.com/web3-storage/web3.storage/tree/main/packages/db/postgres/migrations where when we change SQL code we should create a migration to ease the release by running it. So these changes should be added there
@vasco-santos I'm mainly adding it here to reflect the reality of this table existing in staging/production, and so that the local dev flow creates it so the tests can query against it and pass, and I did that because I thought you, me, and @alanshaw all wanted to make sure that this uploads list api would include backup urls from that backup table in prod. Now, I just want to 'add it' to the code so the local dev and ci/test setup have access to it. I don't want to (re-?)add it in staging/production, because I think it's already there. That's why I didn't add a migration, because I don't want to run a migration on staging/production. I want the localdev to reflect what is running in staging/prod, but only insofar as I can run tests locally that use that table. Does that seem reasonable why there is not a migration or do you still think I should add one @vasco-santos ? |
|
@vasco-santos we never ran a migration to get the data out of the old backup table so it still exists in production. @gobengo just needs it to exist again for local dev and testing so that in production it all works. |
|
Interesting we did not run the migration, the script was created #1305 Perhaps we could just run it now and get rid of backup table instead of making the list more complex? I do not want to block the plan, so if you prefer to go this way is fine for me :) I just though the backup table had been migrated |
|
|
| @@ -22,6 +22,7 @@ const uploadQuery = ` | |||
| created:inserted_at, | |||
| updated:updated_at, | |||
| backupUrls:backup_urls, | |||
| backup( url ), | |||
There was a problem hiding this comment.
I believe you should run https://github.com/web3-storage/web3.storage/blob/main/packages/db/package.json#L13C5-L13C63 to update the types? but I do not remember anymore how this used to work
There was a problem hiding this comment.
Thanks for suggesting this. I wouldn't have known. I ran that script and it produced this diff df36cfb
|
@gobengo does this need to get deployed? |
Website preview 🔗✨ |
gobengo
force-pushed
the
1704839541-car-from-backup-table
branch
from
5a8e97d to
c87f5a0
Compare
|
After deploying to staging via this, I verified |
…ron, api packages (#2362) Motivation: * I want to release the db change from here #2352 in a way that the api also gets released and making use of it * When I merged that last PR, there was no release-please PR generated for api. iiuc, that's because the PR only touched the db package, but not the api package (that depends on db) * hoping that this change that touches the api package.json will release things as desired
Motivation: