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

[ciscolive] Fix issues and dynamically fetch tokens #30865

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

L0wbyte
Copy link

@L0wbyte L0wbyte commented Apr 17, 2022

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

addresses the 3x issues I raised and detailed in #30864

static rainfocus api's in extractor are no longer valid
i have addressed this in the same way the browser does, by fetching the tokens from a js script. It is plausible to only update the static tokens (they have been valid since 2018).
This method may be more useful if the api key updates again, however it equally has the risk of failing if the site javascript dramatically changes.

i have left in the RAINFOCUS_API_PROFILE_ID and RAINFOCUS_WIDGET_ID constants but set them to blank.
If blank the values will be fetched at runtime via a _download_webpage() call.
In future if somebody wished to quickly patch the extractor they could set these values without needing to debug why the dynamic fetch is failing.

tests no longer pass
testing url's and info_dict have been updated with url's and data that are current and pass the tests.

ciscolive domain has changed
updated Origin to match valid domain, updated regex to no longer permit old domain (ciscolive.cisco.com), updated tests with url paths featuring the new domain.

added a couple informative ExtractorError() these occur when tokens can not be obtained or video playlists are empty (the later can occur in 2 scenarios:

  1. where a search returns 0 results (which would result in no download occuring)
  2. where the api key was invalid but the search would have otherwise returned >0 results.

I'm conscious the messages on these ExtractorErrors() could be better/shorter.




technical details for each including failing outputs are contained within issue #30864


Relevant working test outputs

$ python test/test_download.py TestDownload.test_CiscoLiveSession
[CiscoLiveSession] catalog.js: Downloading webpage
[CiscoLiveSession] 16360600004400017rMx: Downloading webpage
[brightcove:new] 6128601216001: Downloading JSON metadata
[brightcove:new] 6128601216001: Downloading m3u8 information
[brightcove:new] 6128601216001: Downloading m3u8 information
[brightcove:new] 6128601216001: Downloading MPD manifest
[brightcove:new] 6128601216001: Downloading MPD manifest
[info] Writing video description metadata as JSON to: test_CiscoLiveSession_6128601216001.info.json
[debug] Invoking downloader on 'https://bcbolt446c5271-a.akamaihd.net/media/v1/pmp4/static/clear/5647924234001/b460c78b-b38e-4aa9-b363-538e45f53415/417dfc89-f5ef-41cb-8af9-6a4a7477c109/main.mp4?akamai_token=exp=1652642459~acl=/media/v1/pmp4/static/clear/5647924234001/b460c78b-b38e-4aa9-b363-538e45f53415/417dfc89-f5ef-41cb-8af9-6a4a7477c109/main.mp4*~hmac=fdf8c03287cdda9b213427413978da4fd903737f11c180bc889b8218524e09fa'
[download] Destination: test_CiscoLiveSession_6128601216001.mp4
[download] 100% of 10.00KiB in 00:00
.
----------------------------------------------------------------------
Ran 1 test in 2.573s

OK
$ python test/test_download.py TestDownload.test_CiscoLiveSearch
[download] Downloading playlist: Search query
[CiscoLiveSearch] catalog.js: Downloading webpage
[CiscoLiveSearch] Downloading search JSON page 1
[CiscoLiveSearch] playlist Search query: Downloading 3 videos
[download] Downloading video 1 of 3
[download] Downloading video 2 of 3
[download] Downloading video 3 of 3
[download] Finished downloading playlist: Search query
.
----------------------------------------------------------------------
Ran 1 test in 0.889s

OK

@dirkf dirkf linked an issue Apr 24, 2022 that may be closed by this pull request
6 tasks
Copy link
Contributor

@dirkf dirkf 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 the work!

youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
youtube_dl/extractor/ciscolive.py Outdated Show resolved Hide resolved
@dirkf
Copy link
Contributor

dirkf commented Apr 28, 2022

Also, check and resolve the failing test output (eg Py3.9 Core):

======================================================================
FAIL: test_no_duplicates (test.test_all_urls.TestAllURLsMatching)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/youtube-dl/youtube-dl/test/test_all_urls.py", line 83, in test_no_duplicates
    self.assertTrue(ie.suitable(url), '%s should match URL %r' % (type(ie).__name__, url))
AssertionError: False is not true : CiscoLiveSearchIE should match URL 'https://www.ciscolive.com/global/on-demand-library.html?search.technicallevel=scpsSkillLevel_aintroductory&search.event=ciscoliveemea2019&search.technology=scpsTechnology_dataCenter&search.focus=scpsSessionFocus_bestPractices#/'

@L0wbyte
Copy link
Author

L0wbyte commented May 1, 2022

Failed tests are now fixed, seems i had left a / off global/ in the CiscoLiveSearchIE _VALID_URL regex.
L0wbyte@9409e00#diff-39087b9fd88e46ad9ea0b1725fce45470a91a8eecccda1891c129b0794ae6e7fR117

$ python3 test/test_all_urls.py TestAllURLsMatching
..............
----------------------------------------------------------------------
Ran 14 tests in 14.689s

OK
$ python3 test/test_execution.py TestExecution.test_lazy_extractors
WARNING: Lazy loading extractors is an experimental feature that may not always work
..............
----------------------------------------------------------------------
Ran 14 tests in 17.109s

OK
.
----------------------------------------------------------------------
Ran 1 test in 18.236s

OK

@L0wbyte L0wbyte requested a review from dirkf May 1, 2022 14:24
Copy link
Contributor

@dirkf dirkf left a comment

Choose a reason for hiding this comment

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

Thanks.

I enabled the CI tests. Please check the results (at least Linter and the core and download tests for Py2.7, Py3.10, possibly Py3.5 too -- for the download tests just search the logs for ciscolive).

@dirkf
Copy link
Contributor

dirkf commented May 15, 2024

Some security snafu has broken the web again, so the CI tests are failing (actions/setup-python#866). I'll see if the recommended hack works for the moment... YES!

@dirkf dirkf changed the title [ciscolive] fix issues and dynamically fetch tokens (closes #30864) [ciscolive] Fix issues and dynamically fetch tokens Jun 13, 2024
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.

[ciscolive] KeyError due to outdated rainfocus api keys and failing tests
2 participants