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

Add document guesser foundations #27

Merged
merged 3 commits into from Aug 25, 2022
Merged

Conversation

lumjjb
Copy link
Contributor

@lumjjb lumjjb commented Aug 24, 2022

Part of #26

Signed-off-by: Brandon Lum lumjjb@gmail.com

Signed-off-by: Brandon Lum <lumjjb@gmail.com>
@lumjjb
Copy link
Contributor Author

lumjjb commented Aug 24, 2022

o wait, adding some simple tests

Signed-off-by: Brandon Lum <lumjjb@gmail.com>
@lumjjb
Copy link
Contributor Author

lumjjb commented Aug 24, 2022

technically, a bit of an edge case where if a document is too small, it may match multiple formats, but don't think that would be an issue.

Copy link
Collaborator

@mihaimaruseac mihaimaruseac left a comment

Choose a reason for hiding this comment

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

LGTM, minor question

return processor.FormatJSON
}
return processor.FormatUnknown
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should call the RegisterDocumentFormatGuesser function here in init, so it always gets registered when this module is imported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added this!

Copy link
Collaborator

@mlieberman85 mlieberman85 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Brandon Lum <lumjjb@gmail.com>
// See the License for the specific language governing permissions and
// limitations under the License.

package guesser
Copy link
Collaborator

Choose a reason for hiding this comment

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

minior nit: Do we want to break out the the implementation into a separate package

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was doing that, then i felt like i was writing java in the 90s, then i reverted it back lol... maybe a follow up PR

@lumjjb lumjjb merged commit 754587c into guacsec:main Aug 25, 2022
mrizzi added a commit to mrizzi/guac that referenced this pull request Nov 16, 2023
* Fix RH CSAF Parser

Signed-off-by: mrizzi <mrizzi@redhat.com>

* CSAF Parser: fix double ingestion

Signed-off-by: mrizzi <mrizzi@redhat.com>

---------

Signed-off-by: mrizzi <mrizzi@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants