Skip to content

Feat: Customize API base URL#73

Merged
hanjano merged 4 commits intomasterfrom
feat/parametrize_base_url
Oct 11, 2022
Merged

Feat: Customize API base URL#73
hanjano merged 4 commits intomasterfrom
feat/parametrize_base_url

Conversation

@hanjano
Copy link
Copy Markdown
Contributor

@hanjano hanjano commented Oct 11, 2022

No description provided.

@hanjano hanjano self-assigned this Oct 11, 2022
Comment thread blockapi/v2/base.py
Comment on lines +83 to +85
"""
Class for handling blockchain API services with customizable base URL.
"""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please add example of common cases (RPC services, testnet alternatives, ...)

Comment thread blockapi/test/v2/api/test_solana.py Outdated
hanjano and others added 2 commits October 11, 2022 13:35
Comment thread blockapi/v2/base.py Outdated
e.g. proxy, testnet, RPC services, alternative sources
"""

API_BASE_URL: str = NotImplemented
Copy link
Copy Markdown
Contributor

@ladovina ladovina Oct 11, 2022

Choose a reason for hiding this comment

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

nitpick: I though this is wrong so I investigated what happens when you use just NotImplemented, looks it works fine (I thought the var will just contain <class NotImplemented> (which is not)... it was not clear to me in the official doc: https://docs.python.org/3/library/constants.html#NotImplemented but this was better: https://s16h.medium.com/pythons-notimplemented-type-2d720137bf41

tldr: probably use of property with raise NotImplementedError would be probably better

@hanjano hanjano merged commit 2fae829 into master Oct 11, 2022
hanjano added a commit that referenced this pull request Oct 11, 2022
Co-authored-by: galvanizze <marek.galvanek@gmail.com>
@hanjano hanjano deleted the feat/parametrize_base_url branch September 1, 2023 09:22
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