-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(json-ocsf): Add new fields for py-ocsf 0.1.0 #3853
Conversation
You can check the documentation for this PR here -> SaaS Documentation |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3853 +/- ##
==========================================
+ Coverage 85.52% 85.55% +0.03%
==========================================
Files 741 741
Lines 22886 22886
==========================================
+ Hits 19573 19580 +7
+ Misses 3313 3306 -7 ☔ View full report in Codecov by Sentry. |
@@ -40,7 +40,7 @@ class FindingOutput(BaseModel): | |||
# Optional since depends on permissions | |||
account_organization_name: Optional[str] | |||
# Optional since depends on permissions | |||
account_tags: Optional[str] | |||
account_tags: Optional[list[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.
I'm not sure about this change since in the CSV this is being shown as a comma separated string like "key1:value1,key2:value2"
. Could you please review that for the CSV? Also check the tests there.
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.
To clarify, this change is fine but we need to verify that we are not breaking anything.
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.
I totally agree with you, the best approach is to get a list of tags for json and tags separated by "," for csv outputs
You can check the documentation for this PR here -> SaaS Documentation |
You can check the documentation for this PR here -> SaaS Documentation |
You can check the documentation for this PR here -> SaaS Documentation |
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.
Great improvement @pedrooot 👏 🚀
You can check the documentation for this PR here -> SaaS Documentation |
Description
New py-ocsf-models version is released and new fields must be added:
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.