added s3 support and fixed invalid range request #82
Conversation
| import { getValidPath } from "./path.js"; | ||
| import { getLogger } from "./logger.js"; | ||
| import { Authorizer } from "./authorizer.js"; | ||
| import { S3 } from '@aws-sdk/client-s3'; |
There was a problem hiding this comment.
I see that the aws dependency has been added as optional. However, when importing this from clean repo and running npm install, this import results in missing dependency error.
src/data.ts:8:20 - error TS2307: Cannot find module '@aws-sdk/client-s3' or its corresponding type declarations.
8 import { S3 } from '@aws-sdk/client-s3';
There was a problem hiding this comment.
Yes, npm install does not install aws
npm run install:aws does (it is in the readme)
I thought you would like to keep the default without aws but I'm happy if the default is with aws
There was a problem hiding this comment.
I prefer to keep the default without aws, but it's not straightforward. If you mark the dependency as optional but then you import it, compilation will fail in the configuration without aws.
Dynamic imports could solve this, but they complicate the code. See e.g.: https://stackoverflow.com/questions/70247008/create-a-typescript-library-with-optional-dependencies-resolved-by-application
Please use the optional dependency only if you can easily implement a dynamic import that allows the code to compile in both scenarios. Otherwise, just include the dependency as required.
There was a problem hiding this comment.
Ah OK, I didn't realize it was making the compilation fail
I already used dynamic import in python (for cuda), and if was pretty simple.I'll check it it works te same way in ts
2 participants
Checklist before merging
CHANGELOG.md