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

Make node diff more readable #1037

Merged
merged 2 commits into from
Dec 19, 2024
Merged

Conversation

gytsto
Copy link
Contributor

@gytsto gytsto commented Dec 18, 2024

Problem

node diff wasnt clear

Solution

make node diff more clear by introducing deepdiff

Mini test to show differences in output:

def test_diff():
    alpha = telio_node(
        identifier="alpha-id",
        public_key="alpha-pub",
        state=NodeState.CONNECTED,
        ip_addresses=["1.1.1.1"],
        allowed_ips=["1.1.1.1/32"],
        nickname="alpha",
        hostname="alpha.nord",
        allow_incoming_connections=True,
        allow_peer_send_files=True,
        path=PathType.DIRECT,
    )
    beta = telio_node(
        identifier="alpha-id",
        public_key="alpha-pub",
        state=NodeState.CONNECTED,
        ip_addresses=["1.1.1.1"],
        allowed_ips=["1.1.1.1/32"],
        nickname="alpha",
        hostname="alpha.nord",
        allow_incoming_connections=True,
        allow_peer_send_files=True,
        path=PathType.DIRECT,
    )
    assert node_diff(alpha, beta) is None
    beta.identifier = "beta-id"
    assert node_diff(alpha, beta) is None

Output before changes:

>       assert node_diff(alpha, beta)
E       assert False
E        +  where False = node_diff(<uniffi.telio_bindings.TelioNode object at 0x7c01e1eebce0>, <uniffi.telio_bindings.TelioNode object at 0x7c01e1eea5a0>)

Output after changes:

>       assert node_diff(alpha, beta) is None
E       assert 'Value of root[\'identifier\'] changed from "alpha-id" to "beta-id".' is None
E        +  where 'Value of root[\'identifier\'] changed from "alpha-id" to "beta-id".' = node_diff(<uniffi.telio_bindings.TelioNode object at 0x7270707bacc0>, <uniffi.telio_bindings.TelioNode object at 0x72707057bc20>)

☑️ Definition of Done checklist

  • Commit history is clean (requirements)
  • README.md is updated
  • Functionality is covered by unit or integration tests

@gytsto gytsto requested a review from a team as a code owner December 18, 2024 13:34
@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from cba1a46 to fca1b3f Compare December 18, 2024 13:35
@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from fca1b3f to 797bf31 Compare December 18, 2024 13:44
Copy link
Contributor

@stalowyjez stalowyjez left a comment

Choose a reason for hiding this comment

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

Imho some comparison in the PR description would be nice

nat-lab/tests/test_events.py Show resolved Hide resolved
@gytsto
Copy link
Contributor Author

gytsto commented Dec 18, 2024

Imho some comparison in the PR description would be nice

updated description

@stalowyjez
Copy link
Contributor

lgtm, +1

@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from cc7c37c to 9bfd848 Compare December 19, 2024 07:06
@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from 9bfd848 to ce29015 Compare December 19, 2024 07:09
@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from ce29015 to ad3cb00 Compare December 19, 2024 08:06
Copy link
Contributor

@tomaszklak tomaszklak left a comment

Choose a reason for hiding this comment

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

+1

@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from ad3cb00 to 8cfd458 Compare December 19, 2024 09:23
@gytsto gytsto enabled auto-merge December 19, 2024 09:24
@gytsto gytsto force-pushed the gytsto/make_node_diff_more_readable branch from 8cfd458 to 7ee739b Compare December 19, 2024 09:33
@gytsto gytsto merged commit 07c7840 into main Dec 19, 2024
64 checks passed
@gytsto gytsto deleted the gytsto/make_node_diff_more_readable branch December 19, 2024 10:43
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.

3 participants