gnocchixyz / gnocchi

Timeseries database
Apache License 2.0
299 stars 85 forks source link

Fix issue with None value attributes #1287

Closed rafaelweingartner closed 1 year ago

rafaelweingartner commented 1 year ago

With Python 3, the sort function is not handling None values. Therefore, when we handle resources with values as None an exception happens. This patch fixes such situations.

rafaelweingartner commented 1 year ago

@tobias-urdin this is also an important one. However, its tests will only pass after the following patches have been merged:

tobias-urdin commented 1 year ago

Could this be reproduced with a test? I'm trying to understand what the change will do to the grouped data, will there be entries with an empty string? Should those be filtered out instead?

rafaelweingartner commented 1 year ago

The code is not designed in a way that enables us to easily do a unit testing. Let me think about what we can do here.

tobias-urdin commented 1 year ago

@mergifyio rebase

mergify[bot] commented 1 year ago

⚠️ This pull request got rebased on behalf of a random user of the organization. This behavior will change on the 1st February 2023, Mergify will pick the author of the pull request instead.

To get the future behavior now, you can configure bot_account options (e.g.: bot_account: { author } or update_bot_account: { author }.

Or you can create a dedicated github account for squash and rebase operations, and use it in different bot_account options.

mergify[bot] commented 1 year ago

rebase

✅ Branch has been successfully rebased

rafaelweingartner commented 1 year ago

@tobias-urdin I added the unit tests for this one, as you suggested.

tobias-urdin commented 1 year ago

Thanks @rafaelweingartner – let me know if you have anything else you want in master I will cut a 4.5 release sometime this week.

rafaelweingartner commented 1 year ago

Thanks @rafaelweingartner – let me know if you have anything else you want in master I will cut a 4.5 release sometime this week.

With respect of Gnocchi server, I think everything is fine. Only the Gnocchi client where I Think there is that adjust in the handling of exceptions that we should discuss a little bit more, and maybe generate a new version of the client as well.