-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[ethPM] Update Infura strategy #1383
[ethPM] Update Infura strategy #1383
Conversation
273f1a9
to
730a8f2
Compare
Assuming there is nothing special about that particular key, we could pull the API key from the environment. That would require that devs have it set locally, and then we would set the env variable in circle. If we do that, it's probably a good idea to add instructions to the README |
tests/ethpm/conftest.py
Outdated
@pytest.fixture | ||
def infura_env(monkeypatch): | ||
# Please play nice and don't use this key for any shenanigans, thanks! | ||
monkeypatch.setenv("WEB3_INFURA_PROJECT_ID", '4f1a358967c7474aae6f8f4a7698aefc') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm ok with this, at least as a temporary solution until we figure out a better approach. You could also use the environment variables and skip+warn when the variable isn't present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
e786a02
to
b272e8c
Compare
b272e8c
to
4fed555
Compare
What was wrong?
ethpm
had its own strategy for connecting to infura via an api key set as an environment variable, which has been updated to useweb3
's strategy. I'd like to get rid of hardcoding the api key, but since we rely on it for testing I'm not sure the best way to achieve that is / if it's even possible.Cute Animal Picture