Infra for setting S3 bucket for storing datasets #147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andresgutgon
force-pushed
the
feature/s3-infra-for-datasets
branch
from
ba3a6b2 to
fe04349
Compare
andresgutgon
commented
Sep 9, 2024
| @@ -1,24 +1,42 @@ | |||
| import { debuglog } from 'node:util' | |||
There was a problem hiding this comment.
This is super nice. It prints debug traces scoped by a string. I saw Flydrive used it
andresgutgon
commented
Sep 9, 2024
| import { Result } from '@latitude-data/core/lib/Result' | ||
| import { env } from '@latitude-data/env' | ||
| import { Disk, errors } from 'flydrive' | ||
| import { FSDriver } from 'flydrive/drivers/fs' | ||
| import { S3Driver } from 'flydrive/drivers/s3' | ||
| import { WriteOptions } from 'flydrive/types' | ||
|
|
||
| var debug_default = debuglog('flydrive:s3') |
There was a problem hiding this comment.
In this case I tag these logs (temp hack to debug) as FlyDrive S3
There was a problem hiding this comment.
var? are we back to 2006 javascript? 
andresgutgon
force-pushed
the
feature/s3-infra-for-datasets
branch
3 times, most recently
from
a8010db to
b6e3ee9
Compare
geclos
reviewed
Sep 9, 2024
| await client.send(command) | ||
| } catch (error) { | ||
| debug_default('error pinging bucket %s', bucket) | ||
| debug_default('error pinging bucket %s', error) |
There was a problem hiding this comment.
shouldn't we throw or do something with the exception here besides printing?
There was a problem hiding this comment.
This method will be removed
geclos
previously approved these changes
Sep 9, 2024
andresgutgon
force-pushed
the
feature/s3-infra-for-datasets
branch
from
4c5a0f4 to
8a9487d
Compare
geclos
force-pushed
the
feature/s3-infra-for-datasets
branch
from
983b1e2 to
c39b0cd
Compare
andresgutgon
force-pushed
the
feature/s3-infra-for-datasets
branch
from
c39b0cd to
81f46a5
Compare
andresgutgon
force-pushed
the
feature/s3-infra-for-datasets
branch
from
81f46a5 to
facceeb
Compare
andresgutgon
commented
Sep 9, 2024
| const adapter = new DrizzlePostgreSQLAdapter(database, sessions, users) | ||
| const adapter = new DrizzlePostgreSQLAdapter( | ||
| // @ts-expect-error - No idea why this is happening | ||
| database, |
There was a problem hiding this comment.
No idea why started failing. Also in main. I'll try after merge and see if I can remove it
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Set S3 bucket for storing datasets in latitude workspaces
TODO
Setup S3 Policy in bucketNot necessary. User fromAWS_ACCESS_KEYalready has these permissions.eu-central-1regionpulumi upto create the bucketNext PR