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

fix(parsers): use_proxy decorator to not override default args #7686

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

Conversation

consideRatio
Copy link
Contributor

@consideRatio consideRatio commented Jan 5, 2025

Description

When WEBPROXY_USERNAME / WEBPROXY_PASSWORD is configured and a use_proxy decorated parser function was called without args/kwargs, like done when using python -m parsers.KPX instead of poetry run test-parsers KR, then the following error showed up:

Error(s) creating production breakdown Event 2025-01-06 07:30:00+09:00: 1 validation error for ProductionBreakdown
zoneKey
  none is not an allowed value (type=type_error.none.not_allowed)

It was because the use_proxy logic always passed a zone_key args explicitly, even if it wasn't received, which made a None value get passed explicitly and short-circuiting the fetch_... functions default value.

Changes

  • Makes calling fetch_production() without args or kwargs not run into issues stemming from zone_key being explicitly passed as None.

  • Adds a comment on what webshare.io subscription is useful for use by this project.

  • Re-configures session.proxies back to what it was in a way that I think now will work, while previously I think we just stored a reference to the proxies object that we then also modified.

    This likely won't impact anyone.

  • Adjusted logging level from Error to Warning. As it kept running without further complaints it felt more like a warning, and it could reasonably succeed without a proxy depending on where the code is running.

  • Adjusted the code for readability, which is an opinionated matter of course.

Double check

  • I have tested my parser changes locally with poetry run test_parser "zone_key"
    Things still work, with and without having WEBSHARE_... environment configured.
  • I have run pnpx prettier@2 --write . and poetry run format in the top level directory to format my changes.

@github-actions github-actions bot added parser python Pull requests that update Python code labels Jan 5, 2025
@consideRatio
Copy link
Contributor Author

consideRatio commented Jan 6, 2025

I'm not sure what the test failure is about, I saw it also in #7687.

Reported via #7689

@consideRatio
Copy link
Contributor Author

consideRatio commented Jan 6, 2025

Force pushed a rebase on master, hoping to see unrelated test failure pass now (see #7689).

@consideRatio consideRatio force-pushed the pr/use-proxy branch 2 times, most recently from e59a06a to 9ca9beb Compare January 6, 2025 20:14
Changes:

- Makes calling `fetch_production()` without args or kwargs not run into
  issues stemming from `zone_key` being explicitly passed as None.
- Adds a comment on what webshare.io subscription is useful for use by
  this project.
- Re-configures session.proxies back to what it was in a way that I
  think now will work, while previously I think we just stored a
  reference to the proxies object that we then also modified.

  This likely won't impact anyone.
- Adjusted the code for readability, which is an opinionated matter of
  course.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant