Skip to content

Conversation

@andig
Copy link
Member

@andig andig commented Nov 1, 2025

TODO

@andig andig added the infrastructure Basic functionality label Nov 1, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@andig andig marked this pull request as draft November 2, 2025 12:16
@andig
Copy link
Member Author

andig commented Nov 2, 2025

@Maschga I was expecting this PR to be a no-up. Could you figure out why the tests break? Maywe we've not providing the JSON expected after all?

@Maschga
Copy link
Collaborator

Maschga commented Nov 2, 2025

The problem is that now GET /api/sessions returns chargeDuration with the unit seconds and previously it had the unit nanoseconds.
No idea why this happens.

@andig
Copy link
Member Author

andig commented Nov 2, 2025

No idea why this happens.

Because I've generally changed this to match precondition and others. Could I ask you to open a new PR to export duration in seconds in order to do this consistently everywhere?

@Maschga
Copy link
Collaborator

Maschga commented Nov 2, 2025

I don't quite understand what you mean by export duration in seconds.
Have you not yet changed duration from int64 to time.Duration?
Should I correct the tests? Or is it something else?

@andig
Copy link
Member Author

andig commented Nov 2, 2025

With this PR- and in future- we will always un/marshal time.Duration in JSON in seconds. Today, it‘s mixed- planner in seconds, sessions in ns. To fix this, we can make it consistent today or with this PR. In both cases, we‘ll need to change the sessions behaviour of the UI wich expects nano-seconds atm.

@github-actions github-actions bot added the stale Outdated and ready to close label Nov 10, 2025
@andig andig added backlog Things to do later and removed stale Outdated and ready to close labels Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backlog Things to do later infrastructure Basic functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants