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

Switch instrument search to use the POST api url. #114

Merged
merged 3 commits into from
Sep 1, 2024

Conversation

tjader
Copy link
Contributor

@tjader tjader commented Aug 23, 2024

This should resolve #113.

@Qluxzz
Copy link
Owner

Qluxzz commented Aug 30, 2024

The return models of the old and new URL are not compatible so this is also a breaking change.

Running the test suite python -m unittest confirms this.
We'd need to update the different return models to match the new ones.

@tjader
Copy link
Contributor Author

tjader commented Aug 31, 2024

The search results all have the same output, so I merged them to use the same model. Also fixed a small issue in the transactions model (and added some bits into .gitignore that shouldn't be committed)

@Qluxzz
Copy link
Owner

Qluxzz commented Sep 1, 2024

LGTM, thanks for updating the tests and models!

@Qluxzz Qluxzz merged commit fa2f5d5 into Qluxzz:master Sep 1, 2024
class SearchPrice(BaseModel):
# Most values are returned as strings.
# null values are returned as "null"
last: str|None
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that this code makes the library incompatible with Python 3.9 (the minimum version stated in setup.py). See https://docs.python.org/3.10/whatsnew/3.10.html#pep-604-new-type-union-operator .

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.

Instrument search broken
3 participants