Skip to content

A few changes to the original code#23

Open
AnakinFoxe wants to merge 11 commits into
andyjsmith:masterfrom
AnakinFoxe:master
Open

A few changes to the original code#23
AnakinFoxe wants to merge 11 commits into
andyjsmith:masterfrom
AnakinFoxe:master

Conversation

@AnakinFoxe

@AnakinFoxe AnakinFoxe commented Mar 28, 2024

Copy link
Copy Markdown

First of all, thanks for this great script. It helped me download all the albums for my kid 😆

Made a few changes to the original code:

  • Authenticate with password (with -p PASSWORD option), however if session ID is provided, password will be ignored.
  • Add an option --folder to download all the albums under the specific folder
  • Concurrently download images of the same album
  • A few useful refactors (I think)
    • Split codes into functions and organize under class SmugMugDownloader
    • Use one session for all the requests
    • Use logging
    • Use @retry to manage retry for get_json()
    • Use Makefile to setup a virtual environment to work with

@andyjsmith

Copy link
Copy Markdown
Owner

Sorry for the late reply, I appreciate the changes!

I'm a bit hesitant on the password authentication due to some unhandled edge cases (e.g. what if the user has 2FA enabled?). Maybe we could keep both the user/pass and session token auth mechanisms instead of replacing it. As for the Makefile, I would like to keep this script platform-independent so I don't want to merge in a Unix-specific script. At some point a setup.py / pyproject.toml may be appropriate.

The other contributions are great but they probably need to be split off as separate PRs.

@sigma-cache sigma-cache left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This repo is a lifesaver. Good point on allowing 2 separate modes Andy.

@AnakinFoxe

Copy link
Copy Markdown
Author

@andyjsmith thanks for the comments! I have updated the PR with following changes (updated the PR description as well)

  • Kept the -s SESSION argument. If session ID is provided, password will be ignored.
  • Removed Makefile
  • README.md has been updated accordingly.

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.

3 participants