Skip to content

feat: Add a warning about clearing command metadata being unsupported#3213

Merged
Bravo555 merged 1 commit intothin-edge:mainfrom
Bravo555:fix/2739/error-message-eof-while-parsing
Oct 30, 2024
Merged

feat: Add a warning about clearing command metadata being unsupported#3213
Bravo555 merged 1 commit intothin-edge:mainfrom
Bravo555:fix/2739/error-message-eof-while-parsing

Conversation

@Bravo555
Copy link
Copy Markdown
Member

Proposed changes

Replaces ERROR: Mapping error: EOF while parsing a value at line 1 column 0 error log with a more meaningful warning log when publishing an empty message to a command metadata topic.

There are some unresolved questions wrt. clearing command metadata (see discussion in #2739), so for now we just provide a helpful message to the user. This feature will be implemented after the questions are resolved.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#2739

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

This PR was created so that the relevant issue can be closed, because the error message itself is not really a problem or a bug, but there's a much bigger design question about how clearing command capabilities should work, primarily when it comes to clearing capabilities of builtin agent commands. As such this PR provides a more meaningful warning to any users, while we can discuss the specifics. For that we can create a new, more design-focused issue about the capability clearing mechanism.

Replaces `ERROR: Mapping error: EOF while parsing a value at line 1
column 0` error log with a more meaningful warning log when publishing
an empty message to a command metadata topic.

There are some unresolved questions wrt. clearing command metadata (see
discussion in thin-edge#2739), so for now we just provide a helpful message to
the user. This feature will be implemented after the questions are
resolved.

Signed-off-by: Marcel Guzik <marcel.guzik@inetum.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/extensions/c8y_mapper_ext/src/converter.rs 66.66% 2 Missing ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek left a comment

Choose a reason for hiding this comment

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

Approved. Indeed, a nice improvement from a user perspective.

@github-actions
Copy link
Copy Markdown
Contributor

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
525 0 2 525 100 1h31m35.311788999s

@Bravo555 Bravo555 added this pull request to the merge queue Oct 30, 2024
Merged via the queue into thin-edge:main with commit bce70ed Oct 30, 2024
@reubenmiller reubenmiller changed the title Add a warning about clearing command metadata being unsupported feat: Add a warning about clearing command metadata being unsupported Dec 16, 2024
@reubenmiller reubenmiller added the theme:mqtt Theme: mqtt and mosquitto related topics label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

theme:mqtt Theme: mqtt and mosquitto related topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants