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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Remove workaround to convert ports to integers #72

Merged
merged 2 commits into from Nov 24, 2020
Merged

Remove workaround to convert ports to integers #72

merged 2 commits into from Nov 24, 2020

Conversation

0snap
Copy link
Contributor

@0snap 0snap commented Nov 23, 2020 •
edited

馃摂 Description

With the merge of tenzir/tenzir#1187 VAST now exports ports a plain integers.
This PR removes a work-around that converted the former VAST-internal representation to ints.

馃摑 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/threatbus, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

馃幆 Review Instructions

Run the modified unit-test suite.

@0snap 0snap added the enhancement An improvement of existing code label Nov 23, 2020
@0snap 0snap requested review from lava and mavam November 23, 2020 16:28
mavam
mavam approved these changes Nov 23, 2020
View reviewed changes
Copy link
Member

@mavam mavam left a comment

Choose a reason for hiding this comment

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

LGTM. Just one left-over comment.

I didn't test this locally, but if this path is triggered by CI, I'm fine with automated testing.

apps/vast/pyvast_threatbus/test_message_mapping.py Outdated Show resolved Hide resolved
@0snap @mavam
Apply suggestions from code review
031584e 
Co-authored-by: Matthias Vallentin <matthias@tenzir.com>
@0snap 0snap merged commit 311e8d0 into master Nov 24, 2020
@0snap 0snap deleted the story/ch1744/remove-port-workaround branch November 24, 2020 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mavam mavam mavam approved these changes

@lava lava Awaiting requested review from lava

Assignees
No one assigned
Labels
enhancement An improvement of existing code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@0snap @mavam