thibaudgg
Get embed URL for unlisted Vimeo videos
When a Vimeo video has its privacy settings set to "unlisted" ("Private on your account, but embeddable anywhere"), the embed code requires an ?h= query parameter. Without it, the player will encounter a 403. The current release of VideoInfo does not obtain this query parameter for unlisted Vimeo videos.
Luckily the Vimeo API's GET /videos/:video_id endpoint return a response containing a player_embed_url property which does contain the appropriate query parameter. Therefore if the host app has configured VideoInfo with a Vimeo API key, we can discover the correct embed URL.
Since this means the embed_url can sometimes contain a query parameter, the base _embed_url method can no longer simply append additional query parameters. Therefore this commit updates the base method to use the URI class to merge parameters.
Notes:
- I see in the contributing guidelines that you want us to ensure consistent styling with
bundle exec standardrb. However I see that there are a few inconsistencies that predate this PR. I'm happy to fix them in another PR if you want!
Hey, thanks for your contribution. 👍🏻 That seems fine, but it would be great to add a spec to cover this change. It might require some VCR recordings update, I haven't worked on this project for quite a while, so not sure what's required anymore.
I see in the contributing guidelines that you want us to ensure consistent styling with bundle exec standardrb. However I see that there are a few inconsistencies that predate this PR. I'm happy to fix them in another PR if you want!
Right, updating standardrb and fixing the styling on a separate PR would be awesome. Thanks!
@thibaudgg I'm a little stuck here. It does seem like all of the cassettes are older than the 6 month max age, so VCR wants to re-record all of them. But it seems like a valid Vimeo PAT would be needed in order to do that. This line in the spec seems to ask for an environment variable VIMEO_ACCESS_TOKEN in order to proceed. Otherwise it will use the literal vimeo_access_token, which will produce 401 errors.
Do you have any advice for resolving this? I could add test coverage and just leave the re-recording to another ticket.
Do you have any advice for resolving this? I could add test coverage and just leave the re-recording to another ticket.
Is getting a VIMEO_ACCESS_TOKEN (from the README: https://developer.vimeo.com/api/guides/start) not working anymore? I would recommend getting a real token and try to re-record all of them, hopefully there isn't too many changes 😬, otherwise, yes this need to be addressed in another ticket/PR.
Thanks for looking into it!
Apologies, I was actually misunderstanding what was happening. I did provide my own API key but I was seeing errors. I thought this was because my token did not grant access to the videos uploaded by other Vimeo users. In reality, the issue is that the scraper is having some trouble accessing video pages. It seems like Vimeo is protected by Cloudflare Turnstile, so VCR's attempts to re-record cassettes ends with a bunch of HTML pages saying "Checking if the site connection is secure." Kind of a road bump...
Additionally it looks like there may be some issues updating the cassettes for password-protected videos using the scraper. The Vimeo scraper's available? method looks for an element with the CSS class exception_title--password. But I think that Vimeo no longer uses that design. I haven't yet been able to locate a nicer way to identify password-protected videos.
I'll have a think on what could be done to get around all this. If you have any thoughts, please let me know! In the meantime, I may just add test coverage for my change to round off this PR.
Edit: I may have gotten somewhere with #234 . If that PR can be reviewed/merged first, I think that will help this one more further along.
Great, I just merged #234, adding specs for this PR shouldn't now be much easier. Thanks! 🙌🏻
@thibaudgg Sorry for the delay here. It looks like there are quite a lot of errors with the DailyMotion and YouTube providers' tests, which I didn't touch. I can try to look into it, but I'm less familiar with those providers, so it will take me some time.
Separately, the linter on ruby 3.4 flags errors for which the solutions are incompatible with Ruby <3.2. I linted this codebase in aa710a7, which is why we're now seeing that the CI is failing for Ruby 2.7, 3.0, and 3.1. How would you like me to resolve this? Those 3 versions are past EOL, so I would vote for removing from your officially supported versions.
@thibaudgg Sorry for the delay here. It looks like there are quite a lot of errors with the DailyMotion and YouTube providers' tests, which I didn't touch. I can try to look into it, but I'm less familiar with those providers, so it will take me some time.
No worries, take all the time you need, thanks for looking into this. 👍🏻
Separately, the linter on ruby 3.4 flags errors for which the solutions are incompatible with Ruby <3.2. I linted this codebase in aa710a7, which is why we're now seeing that the CI is failing for Ruby 2.7, 3.0, and 3.1. How would you like me to resolve this? Those 3 versions are past EOL, so I would vote for removing from your officially supported versions.
Sure, let's not bother with past EOL Ruby versions, we can drop support and release a new major version of the gem.
Hey @thibaudgg, I've update this PR with the following changes:
- Drop support for Ruby <=3.1, which is past EOL
- Add support for Ruby 4
- Mostly this is just adding Logger as a dependency, since it is no longer part of Ruby core
- Update CI
- Run rspec in Ruby v3.2, 3.3, 3.4, and 4.0
- Run standardrb in Ruby v4.0
- Add
.standardrb.ymlto ensure local linting matches CI - Use bundler v4
- Upgrade dependencies to latest versions
- Patch Youtube Scraper's
view_countmethod. Youtube has multiplemeta[itemprop=userInteractionCount]elements on the page. One is for view count, another is for the number of users who have "thumbs up" the video. This PR updates the scraper'sview_countmethod to pick from the correct meta tag. - Patch Youtube Scraper's
descriptionmethod to account for the way YouTube handle videos w/o a description (i.e. it inserts the default YT description) - Re-record all VCR cassettes to ensure integration with current APIs
- Update DailyMotion and YouTube specs to get around changed file names
I have run the tests locally on Ruby versions 3.2.10, 3.3.10, 3.4.8, and 4.0.1. I do not see any failing tests. Despite my local success, as you can tell, the CI is still failing. Would you mind confirming that the YOUTUBE_API_KEY and VIMEO_ACCESS_TOKENS in the Github Actions secrets are still valid? It seems like the Youtube one might be expired or something
PS: Sorry this small patch has turned into a 183 file PR! I can cherry-pick into smaller PRs if that would be your preference.
Wow such an amazing work @tylerpaige thank you so much to make everything up to date 👏 I made quite similar work a couple years ago and it was hard 😅
I think the problem could be related to the fact that secrets are only available for repo maintainers 🤔?
@tylerpaige awesome, thanks for looking into this 🙌🏻
I think that makes sense to address the Ruby / CI updates in a separate PR, so we don't mix it with this one.
Regarding the YOUTUBE_API_KEY and VIMEO_ACCESS_TOKENS tokens, the issue is indeed (if I remember correctly) that GitHub actions triggered from a fork don't have access to the secrets, which is a bummer. There might be some better way available to handle it now. In the meantime, I happily share them with you via direct message (see https://thibaud.gg for my contact) so you can set it on your fork.