Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create execution and state developement guidelines (error handling, return values) #19797

Closed
ryan-lane opened this issue Jan 16, 2015 · 7 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Documentation Relates to Salt documentation P2 Priority 2 Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-low 4th level, cosemtic problems, work around exists stale
Milestone

Comments

@ryan-lane
Copy link
Contributor

We've been having a difficult time coming to a proper agreement on how our execution modules should handle return values and exceptions. If we allow exceptions to propagate, then the state modules will catch the exceptions and display them in the failed states, which is really helpful, but it also means that when you call the execution module from another module, an undocumented and unknown exception will propogate there as well. In some modules we always return a tuple with the return value and an error message, so that there's a consistent return value and the message can still be propagated.

There's pros and cons for each approach, but if would be nice if there was upstream guidelines so that we can have a known consistent behavior across modules.

@jfindlay
Copy link
Contributor

@ryan-lane, although this may not be directly related to your issue, there is an open issue for normalizing exit codes in #18510 for beryllium.

@basepi basepi changed the title Create execution and state developement guidelines Create execution and state developement guidelines (error handling, return values) Jan 20, 2015
@rallytime rallytime added Documentation Relates to Salt documentation Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Jan 20, 2015
@rallytime rallytime added this to the Beryllium milestone Jan 20, 2015
@rallytime
Copy link
Contributor

Yes, it would be good to define a "best-practices" here. Thanks @ryan-lane.

@arnisoph
Copy link
Contributor

+1

@meggiebot meggiebot modified the milestone: Beryllium Jun 18, 2015
@jfindlay jfindlay modified the milestone: Approved Jun 19, 2015
@ssgward ssgward added P2 Priority 2 severity-low 4th level, cosemtic problems, work around exists labels Jun 19, 2015
@meggiebot meggiebot added the Platform Relates to OS, containers, platform-based utilities like FS, system based apps label Jun 22, 2015
@jfindlay jfindlay added the Bug broken, incorrect, or confusing behavior label Sep 8, 2015
@stale
Copy link

stale bot commented Mar 30, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Mar 30, 2018
@damon-atkins
Copy link
Contributor

.

@stale
Copy link

stale bot commented Mar 31, 2018

Thank you for updating this issue. It is no longer marked as stale.

@stale stale bot removed the stale label Mar 31, 2018
@stale
Copy link

stale bot commented Jul 14, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jul 14, 2019
@stale stale bot closed this as completed Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Documentation Relates to Salt documentation P2 Priority 2 Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged Platform Relates to OS, containers, platform-based utilities like FS, system based apps severity-low 4th level, cosemtic problems, work around exists stale
Projects
None yet
Development

No branches or pull requests

8 participants