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

Add price endpoint #22

Merged
merged 7 commits into from
Nov 28, 2023
Merged

Add price endpoint #22

merged 7 commits into from
Nov 28, 2023

Conversation

Uxio0
Copy link
Member

@Uxio0 Uxio0 commented Nov 20, 2023

@Uxio0 Uxio0 force-pushed the add-price-endpoint branch from 236f32f to 6307d31 Compare November 23, 2023 13:36
@Uxio0 Uxio0 marked this pull request as ready for review November 23, 2023 13:39
DEBUG=0
DJANGO_SETTINGS_MODULE=config.settings.test
DJANGO_SECRET_KEY=t3st-s3cr3t#-!k3y
ETHEREUM_NODES_URLS=https://ethereum.publicnode.com,https://polygon-rpc.com
Copy link
Member

Choose a reason for hiding this comment

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

Can this be an array? just to be more specific [https://ethereum.publicnode.com, ...]

Copy link
Member Author

Choose a reason for hiding this comment

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

No, enviroment variables cannot be arrays, they usually are comma separated values.

We are currently using python-environ to parse them, take a look at https://pypi.org/project/python-environ/

DEBUG=0
DJANGO_SETTINGS_MODULE=config.settings.test
DJANGO_SECRET_KEY=t3st-s3cr3t#-!k3y
ETHEREUM_MAINNET_NODE=https://ethereum.publicnode.com
Copy link
Member

Choose a reason for hiding this comment

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

Which is the difference between ETHEREUM_MAINNET_NODE and ETHEREUM_NODES_URLS, also why ETHEREUM_MAINNET_NODE is not in the .env.sample?

Copy link
Member Author

@Uxio0 Uxio0 Nov 27, 2023

Choose a reason for hiding this comment

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

ETHEREUM_MAINNET_NODE is only used for tests, a ethereum mainnet node is required for e2e price oracle tests (same as we do for safe-eth-py or safe-transaction-service)

## Configuration

Environment variables:
- `ETHEREUM_NODES_URLS`: Comma separated list of the node RPCS for the chains supported for fetching prices.
Copy link
Member

Choose a reason for hiding this comment

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

ETHEREUM_MAINNET_NODE is not mentioned here.

SWAGGER_SETTINGS = {
"SECURITY_DEFINITIONS": {
"api_key": {"type": "apiKey", "in": "header", "name": "Authorization"}
},
}

# Ethereum
ETHEREUM_NODE_URL = env("ETHEREUM_NODE_URL", default=None)
Copy link
Member

Choose a reason for hiding this comment

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

ETHEREUM_NODE_URL is also ETHEREUM_MAINNET_NODE?

COPY . .
# /nginx mount point must be created before so it doesn't have root permissions
# ${APP_HOME} root folder will not be updated by COPY --chown, so permissions need to be adjusted
RUN groupadd -g 999 python && \
Copy link
Member

Choose a reason for hiding this comment

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

Maybe is better 1000 here, what do you think?

@@ -0,0 +1,8 @@
CRYPTO_KITTIES_CONTRACT_ADDRESSES = {
"0x06012c8cf97BEaD5deAe237070F9587f8E7A266d", # Mainnet
"0x16baF0dE678E52367adC69fD067E5eDd1D33e3bF", # Rinkeby
Copy link
Member

Choose a reason for hiding this comment

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

Rinkeby still makes sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

That constants file is not required at all, thanks for noticing

@Uxio0 Uxio0 changed the title Add price endpoint (WIP) Add price endpoint Nov 28, 2023
@Uxio0 Uxio0 merged commit 3f07ea5 into main Nov 28, 2023
3 checks passed
@Uxio0 Uxio0 deleted the add-price-endpoint branch November 28, 2023 09:57
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add price endpoint
2 participants