Skip to content
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

Allow File S3Adapter recieve an async function to retrieve config object #2960

Closed
wants to merge 13 commits into from

Conversation

Nikodermus
Copy link
Contributor

The idea behind this is that the secretAccessKey is not directly accessible, but through a vault which is a async operation.

This way the S3 adapter can be used keeping the secrets, or being created with any kind of async operation in mind.

@changeset-bot
Copy link

changeset-bot bot commented May 13, 2020

🦋 Changeset is good to go

Latest commit: 1cbe0c7

We got this.

This PR includes changesets to release 1 package
Name Type
@keystonejs/file-adapters Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Vultraz
Copy link
Contributor

Vultraz commented May 13, 2020

This feels a bit extraneous. What prevents you from just running the async code in a helper to fetch the value before constructing the S3 adapter?

@Nikodermus
Copy link
Contributor Author

I have my lists that use the S3Adapter in different files, meaning they need the S3Adapter to be constructed by the time it's exported. There's no way to have the export to wait for the async code to be run before exporting the list

@timleslie
Copy link
Contributor

I have my lists that use the S3Adapter in different files, meaning they need the S3Adapter to be constructed by the time it's exported. There's no way to have the export to wait for the async code to be run before exporting the list

I think I agree with @Vultraz on this one. You can always make your files export list-generator functions which can accept the adapter at the appropriate time, rather than relying on the S3Adapter to handle the deferred config generation.

In a more general sense, I'd like to avoid ending up in a place where all of our classes which take config work with object or async function as their argument. It makes it much harder to reason about the correctness of the Keystone library itself, which in turns makes it much more difficult to maintain.

@timleslie timleslie closed this May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants