Skip to content

Conversation

@Marenz
Copy link
Contributor

@Marenz Marenz commented May 14, 2025

  • Remove all cert/tls hacks and update official URL
  • Fix forgotten renames to DispatchApiClient
  • Update readme to be more user-friendly

Copilot AI review requested due to automatic review settings May 14, 2025 08:59
@Marenz Marenz requested review from a team as code owners May 14, 2025 08:59
@github-actions github-actions bot added part:docs Affects the documentation part:cli Affects the command-line interface part:dispatcher labels May 14, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prepares the release by removing deprecated certificate/tls hacks, updating the API URL, and renaming the client to DispatchApiClient.

  • Removed custom certificate options from SslOptions in the dispatch client
  • Updated default server URLs in _client.py and main.py
  • Revised examples and documentation in the README and RELEASE_NOTES for clarity and consistency

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

File Description
src/frequenz/client/dispatch/_client.py Removed certificate parameters and updated the API URL
src/frequenz/client/dispatch/main.py Updated the default dispatch API URL
RELEASE_NOTES.md Clarified the description for end_time handling issue
README.md Updated client name, server URL, and documentation link
Comments suppressed due to low confidence (2)

README.md:42

  • [nitpick] The documentation link anchor 'ApiDispatchClient' does not match the updated client name 'DispatchApiClient'; please verify that the anchor accurately reflects the intended naming.
For detailed usage and advanced features, check out the [client documentation](https://frequenz-floss.github.io/frequenz-client-dispatch-python/latest/reference/frequenz/client/dispatch/#frequenz.client.dispatch.ApiDispatchClient).

src/frequenz/client/dispatch/_client.py:78

  • Ensure that the removal of the certificate loading does not compromise TLS verification; if relying solely on 'enabled=True', verify that the underlying TLS library provides the necessary certificate validation.
ssl=SslOptions(enabled=True),

## Bug Fixes

* `end_time` was not correctly handling the `None` value when converted from protobuf to pythons `Dispatch` class.
* Fix that a user might see invalid values for dispatches without `end_time`. It was not correctly handling the `None` value when converted from protobuf to pythons `Dispatch` class.
Copy link

Copilot AI May 14, 2025

Choose a reason for hiding this comment

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

[nitpick] There's a typo in 'pythons'; consider updating to 'Python's' for clarity.

Suggested change
* Fix that a user might see invalid values for dispatches without `end_time`. It was not correctly handling the `None` value when converted from protobuf to pythons `Dispatch` class.
* Fix that a user might see invalid values for dispatches without `end_time`. It was not correctly handling the `None` value when converted from protobuf to Python's `Dispatch` class.

Copilot uses AI. Check for mistakes.
@Marenz Marenz force-pushed the remove-custom-certs branch 2 times, most recently from 15fdc03 to 511e644 Compare May 14, 2025 09:02
Marenz added 3 commits May 14, 2025 11:06
As we decided to do a hard-switch rather than a smooth one,
we are removing all hacks and set it up to work only with the
official domain & certificate now.

Signed-off-by: Mathias L. Baumann <[email protected]>
Signed-off-by: Mathias L. Baumann <[email protected]>
@Marenz Marenz force-pushed the remove-custom-certs branch from 511e644 to 3af7b9f Compare May 14, 2025 09:06
@Marenz Marenz added this pull request to the merge queue May 14, 2025
Merged via the queue into frequenz-floss:v0.x.x with commit 02a3d6d May 14, 2025
5 checks passed
@Marenz Marenz deleted the remove-custom-certs branch May 14, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:cli Affects the command-line interface part:dispatcher part:docs Affects the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants