joeyagreco / pymfl

Python wrapper for the MyFantasyLeague API.
https://pypi.org/project/pymfl/
MIT License
8 stars 2 forks source link

Requests session #11

Open geoffbeier opened 2 years ago

geoffbeier commented 2 years ago

Let callers pass in a requests session object, and use that for cookie storage instead of passing around the MFL_USER_ID cookie. Fixes #10

joeyagreco commented 2 years ago

Thanks for the PR!

My initial thought is why not have the session setup:

    try:
        mfl_user = os.environ["MFL_USER"]
        mfl_pass = os.environ["MFL_PASS"]
        mfl_league = os.environ["MFL_LEAGUE"]
        mfl_ua = os.environ["MFL_USER_AGENT"]
        mfl_cache = os.environ["MFL_CACHE"]
        mfl_cookies = os.environ["MFL_COOKIES"]
    except KeyError as e:
        logger.error(f"MFL parameters need to be configured in the environment:", e)
        return

    # cache things for 24 hours, for now, to prevent abusive requests while iterating in dev
    expiration_seconds = 60 * 60 * 24
    cookiejar = LWPCookieJar(mfl_cookies)
    if os.path.exists(mfl_cookies):
        cookiejar.load(ignore_discard=True)
    session = requests_cache.CachedSession(cache_name="mfl-cache", expire_after=expiration_seconds)
    session.cookies = cookiejar

done for the user automatically in APIConfig.py, specifically the add_config_for_year_and_league_id method.

This would cut down on the boilerplate code required for the user when using this library.

Also, how would the mfl_cache be initially set?

geoffbeier commented 2 years ago

I think it's a good idea for add_config_for_year_and_league_id() to set up and return a default session object if none is passed in. That would create a dependency on requests-cache, and I thought this PR might be easier to accept without adding a dependency.

It would need to either return the session or add an accessor so that the caller could call save() if they want to persist the cookies between runs.

You'd also need to let the caller pass in either a CookieJar object or a filename for some FileCookieJar.

mfl_cache is a string that requests-cache uses to build a filename for its response cache database. In the config I was using in the sample, if the environment variable is set to "mfl_cache", it creates a file called "mfl_cache.sqlite" in the current working directory.

I'd be happy to take a swing at making add_config_for_year_and_league_id() absorb a good default version of the boilerplate if you don't mind adding requests-cache as a dependency.

joeyagreco commented 2 years ago

I think it's a good idea for add_config_for_year_and_league_id() to set up and return a default session object if none is passed in. That would create a dependency on requests-cache, and I thought this PR might be easier to accept without adding a dependency.

It would need to either return the session or add an accessor so that the caller could call save() if they want to persist the cookies between runs.

You'd also need to let the caller pass in either a CookieJar object or a filename for some FileCookieJar.

mfl_cache is a string that requests-cache uses to build a filename for its response cache database. In the config I was using in the sample, if the environment variable is set to "mfl_cache", it creates a file called "mfl_cache.sqlite" in the current working directory.

I'd be happy to take a swing at making add_config_for_year_and_league_id() absorb a good default version of the boilerplate if you don't mind adding requests-cache as a dependency.

I don't mind adding a dependency that makes the library easier for people to pick up. If you could implement this as you described I would appreciate it greatly!