Skip to content

fix: builtin workflows don't have to be removed#3258

Merged
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:fix/confusing-error-message
Nov 22, 2024
Merged

fix: builtin workflows don't have to be removed#3258
didier-wenzek merged 1 commit intothin-edge:mainfrom
didier-wenzek:fix/confusing-error-message

Conversation

@didier-wenzek
Copy link
Copy Markdown
Contributor

@didier-wenzek didier-wenzek commented Nov 21, 2024

Proposed changes

Exclude builtin workflows from methods persisting workflow definitions that are in-use.

This was leading to confusing error messages as no files are related to builtin workflows.

ERROR tedge_agent::operation_workflows::persist: Fail to remove the workflow copy at /etc/tedge/.agent/workflows-in-use/software_list-builtin.toml: No such file or directory

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

#3251

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

@didier-wenzek
Builtin workflows don't have to be released
69a52a2 
This was leading to confusing error messages as no files are created to
persist builtin workflow definitions.

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
@didier-wenzek didier-wenzek added improvement User value theme:workflows Theme: Workflow engine topics labels Nov 21, 2024
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 21, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ore/tedge_agent/src/operation_workflows/persist.rs 33.33% 4 Missing ⚠️
crates/core/tedge_api/src/workflow/supervisor.rs 42.85% 4 Missing ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

🚨 Try these New Features:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 21, 2024

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
524 0 2 524 100 1h33m13.812181s

albinsuresh
albinsuresh approved these changes Nov 22, 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.

LGTM

@didier-wenzek didier-wenzek added this pull request to the merge queue Nov 22, 2024
Merged via the queue into thin-edge:main with commit 82df8e0 Nov 22, 2024
@didier-wenzek didier-wenzek deleted the fix/confusing-error-message branch November 22, 2024 08:58
@reubenmiller reubenmiller changed the title fix: builtin workflows don't have to be released fix: builtin workflows don't have to be removed Dec 16, 2024
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

@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

improvement User value theme:workflows Theme: Workflow engine topics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@didier-wenzek @albinsuresh