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

Support file objects in the legacy bucket: files.parse.com #2001

Merged
merged 1 commit into from
Jun 8, 2016
Merged

Support file objects in the legacy bucket: files.parse.com #2001

merged 1 commit into from
Jun 8, 2016

Conversation

skinp
Copy link
Contributor

@skinp skinp commented Jun 7, 2016

At some point in the past, parse.com switched buckets/domain for files from files.parse.com to files.parsetfss.com. We need to support the older version too because Parse Server currently has no way of reaching those older files.

Files in files.parsetfss.com all have filenames starting with the prefix "tfss-"
Files in files.parse.com all have filenames starting with a UUID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee-"
Files in Parse Server are all other files and should have filenames starting with a 32 bit hex followed by an underscore: "aaaaaaaabbbbccccddddeeeeeeeeeeee_"

We should make sure never to change the way Parse Server name files or at least never make it a "-" seperated UUID like files.parse.com has.

This should fix #1521.

@codecov-io
Copy link

codecov-io commented Jun 7, 2016

Current coverage is 91.91%

Merging #2001 into master will increase coverage by <.01%

@@             master      #2001   diff @@
==========================================
  Files            91         91          
  Lines          6418       6421     +3   
  Methods        1096       1096          
  Messages          0          0          
  Branches       1346       1347     +1   
==========================================
+ Hits           5899       5902     +3   
  Misses          519        519          
  Partials          0          0          

Powered by Codecov. Last updated by e7e2369...65518cd

@drew-gross
Copy link
Contributor

Awesome, LGTM! We have a reviewer-merge tag that you can use if you want the person who reviews your code to merge it. I find it quite handy and use it for 80-90% of my PRs.

@skinp
Copy link
Contributor Author

skinp commented Jun 8, 2016

Oh good tip, I didn't know that, thanks!

@skinp skinp merged commit db76aa3 into parse-community:master Jun 8, 2016
@barnaclejive
Copy link

barnaclejive commented Jun 9, 2016

I know this question isn't directly related but I have been waiting for this fix for a while and am excited to try it out... how can I update my dependency to pull in this change to my parse-server-example running locally? I have tried updating package.json to get the "latest" code a few ways:

"parse-server": "git://github.com/ParsePlatform/parse-server"
"parse-server": "git://github.com/ParsePlatform/parse-server.git#65518cd"

But when I start up I get:

Error: Cannot find module 'parse-server'

Actually, I always have run into this problem anytime I have tried to point a dependency directly at a repository, such as my own fork of parse-server. Am I missing some basic knowledge of nodejs, package.json, git, or some combination? Any help would be awesome, thanks for your work.

@drew-gross
Copy link
Contributor

Did you run npm install after updating your package.json?

@barnaclejive
Copy link

@drew-gross Correct, I edit package.json then run npm install from my project directory.
Here is something I forgot to mention - when pointing a dependency at a github repository, the resulting myproject/node_modules/parse-server/ directory does not have a lib/ directory.

@skinp
Copy link
Contributor Author

skinp commented Jun 9, 2016

@barnaclejive so what we upload to npm is actually the built version that contains this lib/
We don't have this from source by default, so I suppose when you npm install with latest version of parse-server set, it only install sources and not the built lib/. cding to node_modules/parse-server and running npm run build should make this work.

I don't know how we can instruct npm to run the build steps when installing a package from source.

@barnaclejive
Copy link

Thanks @skinp - that makes sense. I tried this but npm run build is expecting src/ which does not seem to be pulled down when using the repository as a dependency.

parse-server@2.2.11 build /.../myproject/node_modules/parse-server
babel src/ -d lib/

src/ doesn't exist

@jeacott1
Copy link

jeacott1 commented Aug 4, 2016

"We should make sure never to change the way Parse Server name files or at least never make it a "-" seperated UUID like files.parse.com has."

actually this is a problem for legacy apps (including the dashboard!) relying on the dashes in the fileformat (or the length) in order to extract the filename.

whether or not apps should ever have done this is moot. the change in file format breaks things in existing apps, and in the dashboard.

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.

File on files.parse.com (instead of files.parsetfss.com) do not load.
5 participants