Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

added support for graph-video url #358

Merged
merged 1 commit into from
Feb 17, 2015

Conversation

zoharbabin
Copy link

for some reason, the new v4 SDK dropped support for the different API sub domains... which meant that video publishing would use the main api rather than the graph-video subdomain.
This minor fix introduces a change to support the video url.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@SammyK
Copy link
Contributor

SammyK commented Feb 16, 2015

This is a good first step, but depending on the HTTP client you're using you'll get different results.

  • Files need to be posted as multipart/form-data
    • For curl implementations, you'll need to make use of CURLFile class to get the proper encoding.
    • Stream wrappers will break because they always assume application/x-www-form-urlencoded encoding.
    • Guzzle should be able to detect the encoding properly if I remember correctly. You might have to explicitly define multipart/form-data though.
  • The timeouts of 60 seconds are far too low for video uploads. This would need to be updated on all the HTTP client implementations.

Feel free to port over any of the code from #279 which added file upload support to 4.1.

@gfosco
Copy link
Contributor

gfosco commented Feb 17, 2015

I don't mind adding this to 4.0, especially since 4.1 isn't released yet.. If the naive approach here doesn't work, for the reasons Sammy mentions, then 4.1 is the answer.

gfosco added a commit that referenced this pull request Feb 17, 2015
added support for graph-video url
@gfosco gfosco merged commit ba3c9e5 into facebookarchive:4.0-dev Feb 17, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants