Skip to content

Multi file upload #80

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

Closed
wants to merge 1 commit into from
Closed

Conversation

mrloop
Copy link

@mrloop mrloop commented Aug 23, 2017

What Changed & Why

Support multiple file upload that are matched using filePattern.
Backwards compatible, default is old behavior using single index.html file.
Multiple files stored in redis hash using HMSET

Related issues

#55

PR Checklist

  • Add tests
  • Add documentation
  • Prefix documentation-only commits with [DOC]

People

@YoranBrondsema

@mrloop mrloop force-pushed the multi-file-upload branch 2 times, most recently from 93fd6c6 to b232519 Compare August 24, 2017 07:29
@duizendnegen
Copy link
Contributor

What's the use case exactly? In what situation do you want to upload multiple files this way?
Would it be possible for the multiple files to get a different key suffix instead? The concern is that users are caught by surprise by the different data structure on the old key they relied on.

@mrloop mrloop force-pushed the multi-file-upload branch from b232519 to caab01a Compare August 24, 2017 08:49
@mrloop mrloop force-pushed the multi-file-upload branch from caab01a to c2bcc7f Compare August 24, 2017 09:01
Copy link
Member

@achambers achambers left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. Could you please help us understand the use case whereby you require uploading two files to redis?

In order to understand the use case we can better provide feedback on your PR. As Pepijn mentioned, magically changing the redis data structure that is used based on whether the filePattern happens to match multiple files seems less than desirable and not very deterministic.

Please give us some more info so that we can work with you on the best solution, whether it be what is proposed in this PR or some other method. 😄

@mrloop
Copy link
Author

mrloop commented Aug 24, 2017

I want to upload and serve multiple files from the same subdomain over https to works with Safaris appcache. Resources served over https to Safari in the appcache EXPLICIT section need to be from the same origin, that is same protocol, subdomain, domain, and port. Had previously tried serving assets from another subdomain pointing to our CDN, it did not work.
For the low volume app deploying not using a CDN for all the assets is acceptable. Using alias would be one solution, but this results in a lot of boilerplate config and also cumbersome when when adding new resources.

There are a couple of few options to make the redis data structure selection better.

  1. Break backwards compatibility and store a single or many files in a redis hash. Cleaner code but a headache for ember-cli-deploy-redis users updating their server code.
  2. Use a config flag to set wither a single file or multiple files are to be stored.
  3. Add a different key prefix/suffix to key for multiple files.
    • I unsure about this option, it could be a bit confusing having the key change based on the filePattern changing.
    • Another thought is where multiple files are matched you store both the single file index.html using the redis key as it is currently generated, and a redis hash containing all the files using the mutli file redis key.

@achambers
Copy link
Member

@mrloop So I can get a better idea, what are the files that you want to upload? This redis plugin as was intended, and really only ever gets used to upload the index.{html,json} file. What is an example of the other files you want to upload?

Are you saying that you're trying to upload all your assets to redis and then your web server will pull the assets from there to serve them?

@YoranBrondsema
Copy link
Contributor

@mrloop Have you considered including the plugin multiple times, i.e. once for each file you want to upload (see http://ember-cli-deploy.com/docs/v1.0.x/including-a-plugin-twice/)? That is the solution that I ended up using for the issue #55 that you referenced.

@mrloop
Copy link
Author

mrloop commented Aug 29, 2017

I've reworked app / deploy and am now serving majority of assets directly from nginx. Serving 2 files from redis / app server index.html and manifest.appcache, managing the 2 files to be deployed via ember-cli-deploy-redis with aliases.

Closing this PR, however if there is any value to it would be happy to work further on it.

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.

4 participants