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

Changed the way the items are downloaded #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Dragunovam
Copy link

@Dragunovam Dragunovam commented May 19, 2022

This other methods is safer, the code will not break if a component is missing and can't be downloaded.
As you can see below the code fails at line 73, where the seflf_get() method is called on the missing deskshare.webm.
Nesting all the components in a try by using a for loop, seems to be the safest way to do this.

$ python download.py https://**redacted**/playback/presentation/2.3/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384 outu
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/metadata.xml...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/shapes.svg...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/presentation/44bd52a9a4d77104fbb1b7eca7475a453ce1c95c-1644220733306/slide-1.png...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/panzooms.xml...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/cursor.xml...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/deskshare.xml...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/presentation_text.json...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/captions.json...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/slides_new.xml...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/video/webcams.webm...
Downloading https://**redacted**/presentation/b4fbf357166b916dd0460ce69e2c7c09fcd8b55f-1644219908384/deskshare/deskshare.webm...
Traceback (most recent call last):
  File "download.py", line 85, in <module>
    sys.exit(main(sys.argv))
  File "download.py", line 81, in main
    d.download()
  File "download.py", line 73, in download
    self._get('deskshare/deskshare.webm')
  File "download.py", line 38, in _get
    resp = urllib.request.urlopen(req)
  File "/usr/lib/python3.8/urllib/request.py", line 222, in urlopen
    return opener.open(url, data, timeout)
  File "/usr/lib/python3.8/urllib/request.py", line 531, in open
    response = meth(req, response)
  File "/usr/lib/python3.8/urllib/request.py", line 640, in http_response
    response = self.parent.error(
  File "/usr/lib/python3.8/urllib/request.py", line 569, in error
    return self._call_chain(*args)
  File "/usr/lib/python3.8/urllib/request.py", line 502, in _call_chain
    result = func(*args)
  File "/usr/lib/python3.8/urllib/request.py", line 649, in http_error_default
    raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 404: Not Found

This other methods is safer, the code will not break if a component is missing and can't be downloaded.
@valerio-bozzolan
Copy link
Contributor

I'm not the maintainer but this seems good to me. Thanks.

In the future it would be nice to distinguish temporary errors (like HTTP status 5** or timeouts etc.) but this is already an improvement to avoid being stuck by a specific missing file.

@valerio-bozzolan
Copy link
Contributor

I think this pull request will fix this specific bug: #24

@valerio-bozzolan
Copy link
Contributor

I think this pull request also fixes this bug: #2

@valerio-bozzolan
Copy link
Contributor

Can I help in merging this change that fixes two bugs?

Thank you @plugorgau for your opinion

@Dragunovam
Copy link
Author

Hi @valerio-bozzolan, sorry for the late reply. You're free to helpwith the merge. I had a few more changes stashed that apparently solve some other issues but I will open different pull requests for those.

@valerio-bozzolan
Copy link
Contributor

valerio-bozzolan commented Mar 3, 2024

Premising that I've done some code review, and I'm merging what I like in this fork:

https://github.com/valerio-bozzolan/bbb-render

Hoping to be useful, this pull request was merged in that fork, at this point:

valerio-bozzolan@4bb83a5

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