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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mitigate InfluxDB compatibility problems. #94

Merged
merged 1 commit into from Jun 21, 2023
Merged

Conversation

alaendle
Copy link
Contributor

@alaendle alaendle commented May 5, 2023

Unfortunately the v1 compatibility mode of v2 doesn't work as expected in case of error messages. See e.g. influxdata/influxdb#21173

While InfluxDB v1 always shows a error element int the response (see https://docs.influxdata.com/influxdb/v1.3/tools/api/), v2 also emits error messages without an error element (see https://docs.influxdata.com/influxdb/v2.7/api/v1-compatibility/).

E.g. v2 could return something like {"code":"unprocessable entity","message":"failure writing points to database: partial write: points beyond retention policy dropped=1"} - which currently leads to a ClientError exception just showing the message that the error key isn't present and the requested URL (which is pretty useless since the original error message is swallowed).

Therefore a small change to provide a workaround for this pretty ugly situation 馃槄

And for sure we all know that this is a problem of the InfluxDB - but maybe you would like to introduce this pretty small change to provide a workaround; if not, please feel free to just close this PR.

And independently thanks for providing this library 鉂わ笍

Unfortunately the v1 compatibility mode of v2 doesn't work as expected in case of error messages.
See e.g. influxdata/influxdb#21173

While InfluxDV v1 always shows a `error` element int the response (see https://docs.influxdata.com/influxdb/v1.3/tools/api/), v2 also emits error messages without an `error` element (see https://docs.influxdata.com/influxdb/v2.7/api/v1-compatibility/).

E.g. v2 could return something like `{"code":"unprocessable entity","message":"failure writing points to database: partial write: points beyond retention policy dropped=1"}` - which currently leads to a `ClientError` exception just showing the message that the `error` key isn't present and the requested URL (which is pretty useless, since the original error message is swallowed.

Therefore a small change to provide a workaround for this pretty ugly situation 馃槄 

And for sure we all know that this is a problem of the InfluxDB - but maybe you would like to introduce this pretty small change to provide a workaround; if not, please feel free to just close this PR. 

And independently thanks for providing this library 鉂わ笍
@alaendle
Copy link
Contributor Author

@maoe - What's your opinion on this PR?

@maoe
Copy link
Owner

maoe commented Jun 20, 2023

Sorry for the delay. Did you actually test this change with InfluxDB v1 and v2? CI is failing so I wasn't sure if it's tested.

@alaendle
Copy link
Contributor Author

As far as I remember CI failed for another/different reason - I've only tested against v2 IRL, but the change is so marginal that I couldn't image it breaks something in v1 - where according to the spec and my experience the error field is always present. Regarding CI, I will try to provide another PR that fixes it.

@maoe
Copy link
Owner

maoe commented Jun 21, 2023

I've only tested against v2 IRL, but the change is so marginal that I couldn't image it breaks something in v1 - where according to the spec and my experience the error field is always present.

Glad to hear that you've tested it against v2. I agree that this change doesn't break something in v1.

Regarding CI, I will try to provide another PR that fixes it.

That would be great!

I'm gonna merge this PR. I'd like to get the CI fixed before we make a new release on Hackage though.

@maoe maoe merged commit b649ca9 into maoe:develop Jun 21, 2023
0 of 5 checks passed
maoe added a commit that referenced this pull request Jun 29, 2023
v1.9.3

* Mitigate InfluxDB compatibility problems (#94)
* Support GHC 9.4 and 9.6 (#95 and #97)
* Revitalize CI (#96)
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.

None yet

2 participants