Skip to content

Conversation

@mitjap
Copy link
Collaborator

@mitjap mitjap commented Jan 30, 2023

This will allow users to use already configured Bucket or use all possible options that GCS provides.

In my case I have a service that is responsible for handling all the secrets and create a global Storage instance. This way I can reuse that object without repeating any code.

As a side note, at the moment GCS does not emit File object on POST_FINISH event, so it is not possible to access the remote file without external Bucket instance.

@mitjap mitjap requested a review from Murderlon January 30, 2023 21:13
Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're in beta phase we can still publish breaking changes. Wouldn't you say it might be better to only settle on passing a bucket object so we can remove the projectId and keyFilename options? (I suppose we would have to make GCS a peer dependency then).

btw I'm on holiday till February 22 so I'm only checking up on a couple things but won't really be coding/releasing.

@mitjap
Copy link
Collaborator Author

mitjap commented Feb 9, 2023

That is even better. I did not want to change too much, but just add new functionality in somewhat backward compatible way. Peer dependency is always a good option.

Current implementation accepts either a Bucket object or Storage options and Bucket options that are directly passed to GCS library. When we finish with this PR I'd do separate PR for S3 to make similar interface.

Enjoy your holidays.

@Murderlon
Copy link
Member

I understand. But if we want to do breaking changes, I think now is the time. I find the current combination of options a bit confusing as one options cancels out other options.

Enjoy your holidays.

Thank you :)

@Murderlon
Copy link
Member

@mitjap what do you think?

@mitjap mitjap force-pushed the gcs-store-options branch from d62c545 to 74526f2 Compare May 11, 2023 19:48
@mitjap
Copy link
Collaborator Author

mitjap commented May 11, 2023

This PR should be complete now. But GCS tests need to be fixed before we merge this.

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@Murderlon Murderlon merged commit 2d07ce2 into tus:main May 15, 2023
@mitjap mitjap deleted the gcs-store-options branch May 15, 2023 10:02
Murderlon added a commit to mitjap/tus-node-server that referenced this pull request May 16, 2023
* main:
  Bump typescript from 4.9.3 to 5.0.4 (tus#425)
  Bump sinon from 15.0.0 to 15.0.4 (tus#426)
  @tus/gcs-store: allow user to pass bucket instance to GCSStore (tus#388)
  Fix e2e test typecheking (tus#429)
  Update incorrect comment (tus#430)
  @tus/s3-store: add missing error code in termination extension (tus#413)
  @tus/[email protected]
  @tus/s3-store: fix uncaught exception when cancelling an upload (tus#412)
  @tus/[email protected]
  @tus/s3-store: fix uncaught exception when removing files (tus#410)
  @tus/s3-store: add termination extension (tus#401)
  @tus/[email protected]
  Bump @google-cloud/storage from 6.8.0 to 6.9.3 (tus#404)
  Bump http-cache-semantics from 4.1.0 to 4.1.1 (tus#393)
  Bump eslint-config-prettier from 8.5.0 to 8.6.0 (tus#392)
  Bump cookiejar from 2.1.3 to 2.1.4 (tus#383)
  Bump aws-sdk from 2.1269.0 to 2.1325.0 (tus#403)
  @tus/server: refactor and improve request validator (tus#402)
  Fix demo (tus#386)
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.

2 participants