Skip to content

feat: tedge upload c8y#3274

Merged
didier-wenzek merged 9 commits intothin-edge:mainfrom
didier-wenzek:feat/tedge-c8y-upload
Dec 13, 2024
Merged

feat: tedge upload c8y#3274
didier-wenzek merged 9 commits intothin-edge:mainfrom
didier-wenzek:feat/tedge-c8y-upload

Conversation

@didier-wenzek
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek commented Nov 29, 2024 •
edited
Loading

Proposed changes

Extend tedge cli with a tedge upload c8y command to upload a file to Cumulocity.

Upload a file to Cumulocity

The command create a new event for the device,
attach the given file content to this new event,
and return the event ID.

Usage: tedge upload c8y [OPTIONS] --file <FILE>

Options:
      --file <FILE>
          Path to the uploaded file

      --mime-type <MIME_TYPE>
          MIME type of the file content
          
          [default: application/octet-stream]

      --type <EVENT_TYPE>
          Type of the event
          
          [default: tedge_UploadedFile]

      --text <TEXT>
          Text description of the event. . Defaults to "Uploaded file: <FILE>"

      --json <JSON>
          JSON fragment attached to the event
          
          [default: {}]

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

#1315

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

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 29, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 67.25979% with 92 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/core/tedge/src/cli/upload/mod.rs 25.42% 44 Missing ⚠️
crates/core/c8y_api/src/http_proxy.rs 52.83% 25 Missing ⚠️
crates/core/tedge/src/cli/upload/c8y.rs 86.07% 5 Missing and 17 partials ⚠️
crates/core/tedge/src/cli/mod.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 29, 2024 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
549 0 2 549 100 1h22m28.00378s

@didier-wenzek didier-wenzek added the theme:c8y Theme: Cumulocity related topics label Nov 29, 2024
didier-wenzek
didier-wenzek commented Nov 29, 2024
View reviewed changes
albinsuresh
albinsuresh reviewed Dec 3, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@albinsuresh albinsuresh left a comment

Choose a reason for hiding this comment

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

The code looks as straight forward as it can be.

crates/core/tedge/src/cli/mod.rs Outdated

/// Interact with Cumulocity
#[clap(subcommand)]
C8y(c8y::C8yCmd),
Copy link
Copy Markdown
Contributor

@albinsuresh albinsuresh Dec 3, 2024

Choose a reason for hiding this comment

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

Wondering if having this concrete enum variant named C8y would make it difficult to support profiles, since #3262 is merged and the c8y@profile style is preferred over --profile arguments.

Copy link
Copy Markdown
Contributor Author

@didier-wenzek didier-wenzek Dec 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Good point. I will rebase now to assess what are the actual issues.

@reubenmiller reubenmiller added this to the 1.4.0 milestone Dec 5, 2024
@didier-wenzek
Copy link
Copy Markdown
Contributor Author

didier-wenzek commented Dec 5, 2024

This commit dd20201 raises several questions:

  • To upload a file to c8y a child device needs to know its external id. How can this be done. The current implementation is a hack.
  • The C8yEndPoint struct is working correctly only on the main device (asking for instance for a c8y.url). Again the current implementation is a workaround.

@reubenmiller
Copy link
Copy Markdown
Contributor

reubenmiller commented Dec 5, 2024

This commit dd20201 raises several questions:

  • To upload a file to c8y a child device needs to know its external id. How can this be done. The current implementation is a hack.
  • The C8yEndPoint struct is working correctly only on the main device (asking for instance for a c8y.url). Again the current implementation is a workaround.

we could limit this feature to the main device until the entity api is available…or allow the use to manually provide the external id if needed

@didier-wenzek didier-wenzek changed the title feat: tedge c8y upload feat: tedge upload c8y Dec 9, 2024
@didier-wenzek
Copy link
Copy Markdown
Contributor Author

didier-wenzek commented Dec 9, 2024

  • To upload a file to c8y a child device needs to know its external id. How can this be done. The current implementation is a hack.

we could limit this feature to the main device until the entity api is available…or allow the use to manually provide the external id if needed

An optional --device-id parameter has been added. See 3a9a526.

  • If not provided: the file is attached to the main device.
  • To upload a file from a child device, the user must provide the external id of that child device.

@albinsuresh
Copy link
Copy Markdown
Contributor

albinsuresh commented Dec 13, 2024

While testing this further, I noticed one minor niggle: If you try to upload a non-existent file, the event is still created without the attachment. So, pre-validating if the file exits and readable might be good.

@didier-wenzek
Copy link
Copy Markdown
Contributor Author

didier-wenzek commented Dec 13, 2024

While testing this further, I noticed one minor niggle: If you try to upload a non-existent file, the event is still created without the attachment. So, pre-validating if the file exits and readable might be good.

Fixed by 5e456b0

@reubenmiller reubenmiller added the theme:cli Theme: cli related topics label Dec 13, 2024
reubenmiller
reubenmiller approved these changes Dec 13, 2024
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.

Approved. Nice addition and we can close one of the more longer standing tickets

@didier-wenzek
Impl tedge c8y upload
80d3cb2 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Add mTLS support to tedge c8y upload
b82c684 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Add system test
c2bfad6 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Upload a file from a child device
e416123 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Make command naming more consistent
1c30cdb 
The command `tedge c8y upload` has been renamed `tedge upload c8y` to be
consistent with the other commands where c8y is a cloud option not a
sub-command. This also makes less confusing the --profile option used to
chose one cloud profile (when applicable).

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Upload a file on a child device twin
f71d6d0 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Simplify c8y url handling
8381da4 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Check mime-type before uploading a file to c8y
3dd55bd 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek
Document tedge upload c8y
4e4a292 
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek didier-wenzek added this pull request to the merge queue Dec 13, 2024
Merged via the queue into thin-edge:main with commit 6388202 Dec 13, 2024
@didier-wenzek didier-wenzek deleted the feat/tedge-c8y-upload branch December 13, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@albinsuresh albinsuresh albinsuresh approved these changes

@reubenmiller reubenmiller reubenmiller 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

theme:c8y Theme: Cumulocity related topics theme:cli Theme: cli related topics

Projects

None yet

Milestone

1.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@didier-wenzek @reubenmiller @albinsuresh