-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat: adds markdown generation to the rules transform entrypoint #282
Conversation
BREAKING CHANGE: Modifies the existing behavior of the rules transform entrypoint Signed-off-by: Jennifer Power <[email protected]>
@gvauter This code is not completely ready yet, but looking for early feedback. |
Signed-off-by: Jennifer Power <[email protected]>
Logging the traceback string in debug mode helps with development without displaying to users by default. Signed-off-by: Jennifer Power <[email protected]>
Signed-off-by: Jennifer Power <[email protected]>
Signed-off-by: Jennifer Power <[email protected]>
Signed-off-by: Jennifer Power <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good. I left one suggestion on the traceback handling. Let me know what you think.
@@ -278,10 +278,13 @@ def __init__(self, arg: str, msg: str): | |||
|
|||
|
|||
def handle_exception( | |||
exception: Exception, msg: str = "Exception occurred during execution" | |||
exception: Exception, | |||
traceback_str: str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make this an optional kwarg in case someone doesn't implement traceback in their code and only sends the exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gvauter Ask discussed, will merge this as is and we can look at other options for exception handling down the road.
Description
This change alters the way
rules-transform
works currently. Instead of ending the workspace management tasks with OSCAL changes, it ends with Markdown changes. A broader approach necessary for therules-transform
action to be useful on its own because the source of truth for authored model content is Markdown.handle_exception
in separate commit (needed for debugging tests for this PR)Fixes #226
Follow On
Not required but there are a lot of flag options here that would be relatively static (e.g.
markdown_path
) that are used in multiple places. It might be worth exploring using a configuration file for inputs that cannot be automatically populate or do have defaults.Type of change
How has this been tested?
Checklist