intentionet / netconan

netconan - a Network Configuration Anonymizer
Apache License 2.0
145 stars 12 forks source link

Only replace stand-alone AS numbers #54

Closed sfraint closed 6 years ago

sfraint commented 6 years ago

This change is Reviewable

progwriter commented 6 years ago

Reviewed 4 of 4 files at r1. Review status: all files reviewed at latest revision, 2 unresolved discussions.


tests/test_sensitive_item_removal.py, line 158 at r1 (raw file):

    ('asdf{0}_asdf{0}asdf', ['65530']),
    ('anonymize.{} and {}?', ['4567', '1234567']),
    ('{}', ['12345'])

Add test where As number is at the beginning of line, followed by some stuff?


tests/test_sensitive_item_removal.py, line 184 at r1 (raw file):

    ('{}{}', ['1234', '5678']),
    ('{}000', ['1234']),
    ('000{}', ['1234'])

Add test for numbers outside of the accepted range? I.e., larger than 4-byte AS number.


Comments from Reviewable

sfraint commented 6 years ago

Review status: all files reviewed at latest revision, 2 unresolved discussions.


tests/test_sensitive_item_removal.py, line 158 at r1 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…
Add test where As number is at the beginning of line, followed by some stuff?

Done.


tests/test_sensitive_item_removal.py, line 184 at r1 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…
Add test for numbers outside of the accepted range? I.e., larger than 4-byte AS number.

Testing that in test_as_number_invalid below.


Comments from Reviewable

progwriter commented 6 years ago
:lgtm:

Reviewed 1 of 1 files at r2. Review status: all files reviewed at latest revision, 1 unresolved discussion.


tests/test_sensitive_item_removal.py, line 184 at r1 (raw file):

Previously, sfraint (Spencer Fraint) wrote…
Testing that in `test_as_number_invalid` below.

Ah, missed that, sorry.


Comments from Reviewable

dhalperi commented 6 years ago

LGTM if LGT to Victor.


Review status: 1 of 4 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

progwriter commented 6 years ago

Reviewed 3 of 3 files at r3. Review status: all files reviewed at latest revision, 1 unresolved discussion.


netconan/sensitive_item_removal.py, line 42 at r3 (raw file):


class ASNumberAnonymizer:

For the sake of consistency with IpAnonymizer, make this AsNumberAnonymizer?


Comments from Reviewable

sfraint commented 6 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.


netconan/sensitive_item_removal.py, line 42 at r3 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…
For the sake of consistency with `IpAnonymizer`, make this `AsNumberAnonymizer`?

Done.


Comments from Reviewable

dhalperi commented 6 years ago

Reviewed 1 of 1 files at r4. Review status: 2 of 4 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

dhalperi commented 6 years ago

Reviewed 2 of 2 files at r5. Review status: all files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

progwriter commented 6 years ago
:lgtm:

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

progwriter commented 6 years ago

Reviewed 1 of 1 files at r4, 2 of 2 files at r5. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable