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

Minor fix to Accounts "list_transactions" and specifcy working urllib3 version #99

Merged
merged 3 commits into from
Nov 1, 2024

Conversation

Robert-Zacchigna
Copy link
Contributor

@Robert-Zacchigna Robert-Zacchigna commented Oct 25, 2024

Hello @jessecooper, this short series of commits fixes an oversight in the list_transactions function in accounts.py and updated package versions.

CHANGES

  • Updated start/end date params to explicitly need a datetime.date obj
    • The endpoint url always expects a date and NOT datetime
    • I have also modified the payload config accordingly to parse out the datetime in the format expected by the endpoint.
  • Made the latest working version of urllib3 to be 1.26.19
    • The +2.x versions do not work with etrade for whatever reason (or at least its very flaky).
  • Re-locked the package versions of everything to get rid the other automated PRs from dependabot.

HEADS-UP

Also just as a reminder, when you go to bump the package version, I think you forgot to update the pyproject.toml#L3 version at the very top when you bumped the last version from 2.0.1 to 2.1.0.

Thus the release was 2.1.0 but the .toml file was still saying 2.0.1

Pip was unable to resolve the package version as a result. I had to download the package file directly from pypi and manually update the .toml file to the correct version in order to install 2.1.0

So when you bump the version, you'll need to do it in the pyproject.toml file as well as the other locations (same as before).

NOTE: My recommendation for the next version would be 2.1.1 (as this is a very small change).

Let me know if there is anything else you need from me, please review at your earliest convenience.

@jessecooper
Copy link
Owner

@Robert-Zacchigna apologies for the delay looking at this and thank you for pointing out the bump issue. I will review this soon.

Copy link
Owner

@jessecooper jessecooper left a comment

Choose a reason for hiding this comment

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

LGTM;

@jessecooper
Copy link
Owner

@Robert-Zacchigna looks good. I will probably do the bump and tag by Monday.

@jessecooper jessecooper merged commit 209d0d9 into jessecooper:master Nov 1, 2024
12 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