Skip to content

feat: REST API to update entity twin data#3425

Merged
albinsuresh merged 2 commits intothin-edge:mainfrom
albinsuresh:feat/twin-data-rest-api
Mar 6, 2025
Merged

feat: REST API to update entity twin data#3425
albinsuresh merged 2 commits intothin-edge:mainfrom
albinsuresh:feat/twin-data-rest-api

Conversation

@albinsuresh
Copy link
Copy Markdown
Contributor

@albinsuresh albinsuresh commented Feb 26, 2025 •
edited
Loading

Proposed changes

REST API to update entity twin data

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


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

@albinsuresh albinsuresh marked this pull request as draft February 26, 2025 15:38
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 26, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 81.86047% with 39 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/core/tedge/src/cli/http/cli.rs 0.00% 12 Missing ⚠️
crates/core/tedge/src/cli/http/command.rs 0.00% 11 Missing ⚠️
...s/core/tedge_agent/src/http_server/entity_store.rs 92.02% 1 Missing and 10 partials ⚠️
...ates/core/tedge_agent/src/entity_manager/server.rs 89.28% 2 Missing and 1 partial ⚠️
...rates/core/tedge_agent/src/entity_manager/tests.rs 92.30% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 26, 2025 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
589 0 3 589 100 1h38m57.951479s

@albinsuresh albinsuresh force-pushed the feat/twin-data-rest-api branch from 57d1501 to 7f8012a Compare February 27, 2025 06:17
@albinsuresh albinsuresh force-pushed the feat/twin-data-rest-api branch from 7f8012a to e8a524c Compare February 27, 2025 06:45
@albinsuresh albinsuresh marked this pull request as ready for review February 27, 2025 06:46
didier-wenzek
didier-wenzek reviewed Feb 28, 2025
View reviewed changes
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.

  • It would be good to implement tedge http patch along this PR.
  • I was expecting the updated twin data to be published over the twin topics.

@didier-wenzek
Copy link
Copy Markdown
Contributor

didier-wenzek commented Feb 28, 2025

  • It would be good to implement tedge http patch along this PR.

I pushed a commit for that: 4e3e940

@albinsuresh albinsuresh force-pushed the feat/twin-data-rest-api branch from 4e3e940 to 6e7c4c6 Compare March 3, 2025 13:50
@albinsuresh albinsuresh temporarily deployed to Test Pull Request March 3, 2025 13:50 — with GitHub Actions Inactive
@albinsuresh
Copy link
Copy Markdown
Contributor Author

albinsuresh commented Mar 3, 2025

I was expecting the updated twin data to be published over the twin topics.

Resolved by 6e7c4c6

@didier-wenzek
Copy link
Copy Markdown
Contributor

didier-wenzek commented Mar 5, 2025

  • It would be good to implement tedge http patch along this PR.

I pushed a commit for that: 4e3e940

Oops, this commits has been removed by git push --force. It would be better to use git push --force-with-lease

didier-wenzek
didier-wenzek reviewed Mar 5, 2025
View reviewed changes
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.

I will be happy to merge this PR. However, I have first to re-push the commit adding support for tedge http patch.

docs/src/operate/registration/register.md Outdated
},
}
```
* 400: Bad Request
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek Mar 5, 2025

Choose a reason for hiding this comment

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

I would give a bit more context on these errors, even if one can guess it.

Copy link
Copy Markdown
Contributor Author

@albinsuresh albinsuresh Mar 6, 2025

Choose a reason for hiding this comment

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

Resolved by 3e94e0e

@didier-wenzek didier-wenzek temporarily deployed to Test Pull Request March 5, 2025 10:35 — with GitHub Actions Inactive
@didier-wenzek
Copy link
Copy Markdown
Contributor

didier-wenzek commented Mar 5, 2025

I have first to re-push the commit adding support for tedge http patch.

Done: bece5bc

reubenmiller
reubenmiller approved these changes Mar 5, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

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

Nice feature.

As discussed, we could build upon this interface and also look at providing a similar API endpoint which mirrors the MQTT twin topic (te/{topic-id}/twin/{type}) as this would make it easier for users to switch between the MQTT and HTTP interfaces.

didier-wenzek
didier-wenzek approved these changes Mar 5, 2025
View reviewed changes
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. Thin-edge registration over HTTP is now a thing. Nice!

albinsuresh and others added 2 commits March 6, 2025 07:05
@albinsuresh albinsuresh force-pushed the feat/twin-data-rest-api branch from bece5bc to 89c2e3c Compare March 6, 2025 07:05
@albinsuresh albinsuresh temporarily deployed to Test Pull Request March 6, 2025 07:05 — with GitHub Actions Inactive
@albinsuresh albinsuresh enabled auto-merge March 6, 2025 07:06
@albinsuresh albinsuresh added this pull request to the merge queue Mar 6, 2025
Merged via the queue into thin-edge:main with commit 98297f0 Mar 6, 2025
33 checks passed
@albinsuresh albinsuresh deleted the feat/twin-data-rest-api branch March 6, 2025 08:23
@reubenmiller reubenmiller added skip-release-notes Don't include the ticket in the auto generated release notes theme:entity_store Entity store related functionality labels May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reubenmiller reubenmiller reubenmiller approved these changes

@didier-wenzek didier-wenzek didier-wenzek approved these changes

@jarhodes314 jarhodes314 Awaiting requested review from jarhodes314 jarhodes314 is a code owner

@rina23q rina23q Awaiting requested review from rina23q rina23q is a code owner

Assignees

No one assigned

Labels

skip-release-notes Don't include the ticket in the auto generated release notes theme:entity_store Entity store related functionality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@albinsuresh @didier-wenzek @reubenmiller