sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
949 stars 403 forks source link

currency: replace `exchangerate.host` -> `open.er-api.com` #2512

Closed dgw closed 11 months ago

dgw commented 1 year ago

exchangerate.host was bought and changed to require an API key. The new owner also allegedly broke compatibility with the original endpoints, based on GitHub comments from other developers, so I didn't even bother signing up for a key and testing it. I just found a new keyless API.

ExchangeRate-API.com also provides key-based services, but there's an open endpoint that doesn't require any key we can use for now.

VCR file for currency example test has been updated, as well.

Note that this will be a "breaking" change for users who specify the currency.fiat_provider option in their settings as exchangerate.host since that's no longer a valid value. (Ask me how I know... I forgot to update the default at first and the test suite failed because of it. :D)

Checklist

dgw commented 1 year ago

OK, I'm at a bit of a loss. The bundled VCR cassettes work in our CI, but fail for me locally with a weird Brotli error:

_____________________________________________ test_example_exchange_cmd_0 ______________________________________________
/home/dgw/github/sopel/sopel/tests/pytest_plugin.py:148: in test
    ???
E   AssertionError: Output does not match the regex:
E   Pattern: 100 USD is [\d\.]+ BTC, [\d\.]+ CAD, [\d\.]+ EUR, \(unsupported: CAN, AUX\)
E   Output: Something went wrong while I was getting the exchange rate.
-------------------------------------------------- Captured log call ---------------------------------------------------
ERROR    sopel.modules.currency:currency.py:127 Error in GET request: ('Received response with content-encoding: br, but
failed to decode it.', error('BrotliDecoderDecompressStream failed while processing the stream'))

Guessing there's something wrong with my local environment, given that there's also a weird error from mypy when I run make lint (as mentioned in the PR description here) that doesn't happen in CI.

Whoever reviews this, if you have a moment to checkout and test locally, it's not part of the PR but I'd appreciate knowing whether this failure also happens for you locally.

dgw commented 11 months ago

Whoever reviews this, if you have a moment to checkout and test locally, it's not part of the PR but I'd appreciate knowing whether this failure also happens for you locally.

QA passes for me on 3.8-3.11 👍

I should probably just budget time for a nuke-and-rebuild of my local environment before we finalize 8.0. This plus the weird mypy check failure that nobody else sees (gone after #2514) makes me just not trust anything my terminal tells me.

Thanks for checking!