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

All orders start date parameter #517

Merged
merged 6 commits into from
Dec 22, 2024

Conversation

noLeash
Copy link
Contributor

@noLeash noLeash commented Dec 10, 2024

Hello, and I'm excited to submit my first pull request. I have discovered that by adding a parameter of updated_at[gte]=start_date request URL via the arguments for option_orders_url() and orders_url(), it is possible to limit the order request by start date, proportionally speeding up the time required to receive a short list of orders.

I have tested with pytest and confirmed that the results are in an independent test. Results with two different dates are attached.

If you have any feedback or notes on my contribution process, I would be grateful to know if I submitted everything correctly or not.

RS-order-trim_test_results

@noLeash noLeash changed the title All orders start date paramater All orders start date parameter Dec 11, 2024
@mw66
Copy link

mw66 commented Dec 12, 2024

@jmfernandes can you take a look and merge this PR?

@jmfernandes
Copy link
Owner

jmfernandes commented Dec 14, 2024

There’s a small error in the URLs. If both account number and start_date are provided there will be two question marks added to string. There should be only 1 question mark and an & between the values like this

?account=blah&updated_at=blah

@noLeash
Copy link
Contributor Author

noLeash commented Dec 17, 2024

@jmfernandes, I've corrected the url functions for options_order_url() and orders_url() please let me know if there are any other edits required to merge.

@mw66
Copy link

mw66 commented Dec 19, 2024

@jmfernandes can you make a new release after merge this PR? It's an important performance fix.

@jmfernandes jmfernandes merged commit 63ae3fc into jmfernandes:master Dec 22, 2024
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