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

Fixes issues 2301 and 2383 AttributeError: module 'requests.cookies' has no attribute 'update' #2388

Merged
merged 2 commits into from
Mar 30, 2025

Conversation

JanMkl
Copy link
Contributor

@JanMkl JanMkl commented Mar 26, 2025

  • In utils.py line 196 if the session is None then _requests is mistakenly passed to as session to data.py which causes attribute error: module 'requests.cookies' has no attribute 'update'
  • The above condition happens only when searching tickers with ISIN numbers
  • Added tests with ISIN numbers
  • Added tickerBase to raise an ValueError if an empty tickername is passed or isin search results in to empty ticker. This is to have consistent behaviour with utils.get_all_by_isin()

This PR requires that PR #2382 is first merged otherwise new tests introduced for ISIN will fail to same error as #2343 and #2363

@JanMkl JanMkl changed the title Fixes issues 2301 and 2383 Fixes issues 2301 and 2383 AttributeError: module 'requests.cookies' has no attribute 'update' Mar 26, 2025
@JanMkl JanMkl force-pushed the fix-issue-2301 branch 2 times, most recently from 7b79946 to 1387b91 Compare March 26, 2025 20:16
@JanMkl
Copy link
Contributor Author

JanMkl commented Mar 26, 2025

@ValueRaider can you check this?

@ValueRaider
Copy link
Collaborator

ValueRaider commented Mar 26, 2025

#2382 now merged. You can rebase this branch to freshly-pulled dev to "pull in" fix.

Not important

- In utils.py line 165 if the session is None then _requests is
  mistakenly passed to as session to data.py which causes attribute
  error: module 'requests.cookies' has no attribute 'update'
- The above condition happens only when searching tickers with ISIN
  numbers
- Added tests with ISIN numbers
- Added tickerBase to raise an ValueError if an empty tickername is
  passed or isin search results in to empty ticker. This is to have
  consistent behaviour with utils.get_all_by_isin()
- Corrected ruff check failures
- Rebasing to latest upstream/dev
- Updated the value error to contain the ISIN number that wasn't found
@JanMkl JanMkl requested a review from ValueRaider March 26, 2025 21:59
@ValueRaider
Copy link
Collaborator

conflicts

@ValueRaider ValueRaider merged commit a358c88 into ranaroussi:dev Mar 30, 2025
2 checks passed
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