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.

Sign up for GitHub

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

Jump to bottom

make streaming upload default true #5315

Merged
merged 3 commits into from
Jul 10, 2024
Merged

make streaming upload default true #5315

merged 3 commits into from
Jul 10, 2024

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Jul 8, 2024

No description provided.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jul 8, 2024 •
edited
Loading

Diff output files 
diff --git a/packages/@uppy/companion/lib/config/companion.js b/packages/@uppy/companion/lib/config/companion.js
index 1839910..ee2dae0 100644
--- a/packages/@uppy/companion/lib/config/companion.js
+++ b/packages/@uppy/companion/lib/config/companion.js
@@ -20,7 +20,7 @@ const defaultOptions = {
   enableUrlEndpoint: false,
   allowLocalUrls: false,
   periodicPingUrls: [],
-  streamingUpload: false,
+  streamingUpload: true,
   clientSocketConnectTimeout: 60000,
   metrics: true,
 };
diff --git a/packages/@uppy/companion/lib/standalone/helper.js b/packages/@uppy/companion/lib/standalone/helper.js
index 5c3e44d..edaa622 100644
--- a/packages/@uppy/companion/lib/standalone/helper.js
+++ b/packages/@uppy/companion/lib/standalone/helper.js
@@ -177,7 +177,9 @@ const getConfigFromEnv = () => {
     allowLocalUrls: process.env.COMPANION_ALLOW_LOCAL_URLS === "true",
     // cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far.
     cookieDomain: process.env.COMPANION_COOKIE_DOMAIN,
-    streamingUpload: process.env.COMPANION_STREAMING_UPLOAD === "true",
+    streamingUpload: process.env.COMPANION_STREAMING_UPLOAD
+      ? process.env.COMPANION_STREAMING_UPLOAD === "true"
+      : undefined,
     maxFileSize: process.env.COMPANION_MAX_FILE_SIZE ? parseInt(process.env.COMPANION_MAX_FILE_SIZE, 10) : undefined,
     chunkSize: process.env.COMPANION_CHUNK_SIZE ? parseInt(process.env.COMPANION_CHUNK_SIZE, 10) : undefined,
     clientSocketConnectTimeout: process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT

Murderlon
Murderlon reviewed Jul 9, 2024
View reviewed changes
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.

LGTM

packages/@uppy/companion/src/standalone/helper.js
@@ -173,7 +173,7 @@ const getConfigFromEnv = () => {
allowLocalUrls: process.env.COMPANION_ALLOW_LOCAL_URLS === 'true',
// cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far.
cookieDomain: process.env.COMPANION_COOKIE_DOMAIN,
streamingUpload: process.env.COMPANION_STREAMING_UPLOAD === 'true',
streamingUpload: process.env.COMPANION_STREAMING_UPLOAD ? process.env.COMPANION_STREAMING_UPLOAD === 'true' : undefined,
Copy link
Member

Choose a reason for hiding this comment

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

Why true or undefined instead of true/false as before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because we want it to default to true if the env variable is not set ( but it should still be possible to). if we return false here it will override the default which is true afaik

config gets merged here:

uppy/packages/@uppy/companion/src/standalone/helper.js

Line 233 in f75d3f1

return merge({}, getConfigFromEnv(), getConfigFromFile(), options)

@aduh95 aduh95 merged commit 682de7a into 4.x Jul 10, 2024
20 checks passed
@aduh95 aduh95 deleted the companion-default-streaming-upload branch July 10, 2024 13:29
@github-actions github-actions bot mentioned this pull request Jul 10, 2024
Merged
github-actions bot added a commit that referenced this pull request Jul 10, 2024
@github-actions
Release: [email protected] (#5325)
c6c279f 
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/angular             |   0.7.0 | @uppy/onedrive            |   4.0.0 |
| @uppy/audio               |   2.0.0 | @uppy/progress-bar        |   4.0.0 |
| @uppy/aws-s3              |   4.0.0 | @uppy/provider-views      |   4.0.0 |
| @uppy/aws-s3-multipart    |   4.0.0 | @uppy/react               |   4.0.0 |
| @uppy/box                 |   3.0.0 | @uppy/react-native        |   0.6.0 |
| @uppy/companion           |   5.0.0 | @uppy/redux-dev-tools     |   4.0.0 |
| @uppy/companion-client    |   4.0.0 | @uppy/remote-sources      |   2.0.0 |
| @uppy/compressor          |   2.0.0 | @uppy/screen-capture      |   4.0.0 |
| @uppy/core                |   4.0.0 | @uppy/status-bar          |   4.0.0 |
| @uppy/dashboard           |   4.0.0 | @uppy/store-default       |   4.0.0 |
| @uppy/drag-drop           |   4.0.0 | @uppy/store-redux         |   4.0.0 |
| @uppy/drop-target         |   3.0.0 | @uppy/svelte              |   4.0.0 |
| @uppy/dropbox             |   4.0.0 | @uppy/thumbnail-generator |   4.0.0 |
| @uppy/facebook            |   4.0.0 | @uppy/transloadit         |   4.0.0 |
| @uppy/file-input          |   4.0.0 | @uppy/tus                 |   4.0.0 |
| @uppy/form                |   4.0.0 | @uppy/unsplash            |   4.0.0 |
| @uppy/golden-retriever    |   4.0.0 | @uppy/url                 |   4.0.0 |
| @uppy/google-drive        |   4.0.0 | @uppy/utils               |   6.0.0 |
| @uppy/google-photos       |   0.2.0 | @uppy/vue                 |   2.0.0 |
| @uppy/image-editor        |   3.0.0 | @uppy/webcam              |   4.0.0 |
| @uppy/informer            |   4.0.0 | @uppy/xhr-upload          |   4.0.0 |
| @uppy/instagram           |   4.0.0 | @uppy/zoom                |   3.0.0 |
| @uppy/locales             |   4.0.0 | uppy                      |   4.0.0 |

- meta: Bump docker/setup-qemu-action from 3.0.0 to 3.1.0 (dependabot[bot] / #5314)
- meta: Bump docker/build-push-action from 6.2.0 to 6.3.0 (dependabot[bot] / #5313)
- @uppy/core: bring back resetProgress (Merlijn Vos / #5320)
- docs: add note regarding `COMPANION_CLIENT_ORIGINS_REGEX` (Antoine du Hamel / #5322)
- @uppy/companion: make streaming upload default to `true` (Mikael Finstad / #5315)
- docs: change slug for aws docs (Merlijn Vos / #5321)
- @uppy/core: export UppyOptions, UppyFile, Meta, Body (Merlijn Vos / #5319)
- meta: fix React linter rules (Antoine du Hamel / #5317)
- meta: enforce use of extension in import type declarations (Antoine du Hamel / #5316)
- @uppy/companion: remove `oauthOrigin` (Antoine du Hamel / #5311)
- docs,@uppy/companion-client: don't close socket when pausing (Mikael Finstad / #4821)
- @uppy/aws-s3: fix signing on client for bucket name with dots (Antoine du Hamel / #5312)
- @uppy/react: introduce useUppyEvent (Merlijn Vos / #5264)
- @uppy/companion: do not list Node.js 20.12 as compatible (Antoine du Hamel / #5309)
- @uppy/provider-views: `.openFolder()` - return progress indication (Evgenia Karunus / #5306)
- examples,@uppy/companion: Release: [email protected] (github-actions[bot] / #5304)
- @uppy/companion: fix `TypeError` when parsing request (Antoine du Hamel / #5303)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Murderlon Murderlon Murderlon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mifi @Murderlon @aduh95