Skip to content

Conversation

@tfinnell
Copy link
Collaborator

@tfinnell tfinnell commented Sep 9, 2017

Provides the functionality that was requested in issue #28

Videos are handled on a separate subdomain than the standard graph api.

Passing the proper tuples to hackney was the only difficult part about this, but the HTTPoison codebase & documentation helped a bit here. While I was digging in I noticed there's quite a bit of overlapping functionality between HTTPoison and Graph. What are your thoughts on replacing the request/method functions with a dependancy or two?

Wasn't sure if we wanted to add static assets to the project to get the tests to pass, my solution was just to use environment variables [FBEX_PHOTO_FILE, FBEX_VIDEO_FILE] for now so be sure you've got those loaded before running mix test.

@mweibel
Copy link
Owner

mweibel commented Sep 12, 2017

While I was digging in I noticed there's quite a bit of overlapping functionality between HTTPoison and Graph. What are your thoughts on replacing the request/method functions with a dependancy or two?

I'm totally up for that!

Wasn't sure if we wanted to add static assets to the project to get the tests to pass, my solution was just to use environment variables [FBEX_PHOTO_FILE, FBEX_VIDEO_FILE] for now so be sure you've got those loaded before running mix test.

Yeah I guess we'd need those in a test folder because Travis CI will need them too. Is this just the path to such a file?

@tfinnell
Copy link
Collaborator Author

Great, since you're open, I'll make a proposal that introduces HTTPoison in another issue/PR.

Yup, just a path to the media files to be published. I removed the environment variables and just reference the sample files which are now located in test/assets.

@mweibel
Copy link
Owner

mweibel commented Sep 21, 2017

This looks good, will test it on my own and see why travis failed later on. Sorry for the delay in reviewing

@mweibel
Copy link
Owner

mweibel commented Sep 27, 2017

I'm unsure about the graph-video.facebook.com URL because of the following:

  • when I try running the test I always get a timeout
  • it's missing the graph API version which I assume should be there too (according to Uploading Videos Documentation)
  • if I switch to the normal graph API it works, no timeout occurs (docs)

Could it be that the graph-video.facebook.com URL is only for resumable uploads and we'd need to first start the transfer, then another request for uploading and finally one to finish the transfer if we'd want to use that URL?

@tfinnell
Copy link
Collaborator Author

tfinnell commented Oct 8, 2017

graph-video.facebook.com will accept a non-resumable file upload if it's posted as form-data it seems. My comments incorrectly documented that data-structure, I believe. Sorry about that! I simplified the publish inputs for images/videos to receive the file path directly and create the form structure internally.

@mweibel mweibel merged commit f4f3f88 into mweibel:master Oct 9, 2017
@mweibel
Copy link
Owner

mweibel commented Oct 9, 2017

Thanks! Version 0.15.0 has been published on hex.
I invited you to become a collaborator as you've added non significant changes :)

Keep up the good work 👍

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