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 http port in node details #262

Merged
8 commits merged into from
Mar 18, 2022
Merged

Add http port in node details #262

8 commits merged into from
Mar 18, 2022

Conversation

blackode
Copy link
Contributor

@blackode blackode commented Mar 17, 2022

Description

Before this change, there isn't the http_port information in the node content section. So, added the http_port information in the node structure.

Code Changes

New Node struct

   [
    :first_public_key,
    :last_public_key,
    :last_address,
    :reward_address,
    :ip,
    :port,
# --------new field-----------
     :http_port,
# ------------------------------
    :geo_patch,
    :network_patch,
    :enrollment_date,
    available?: false,
    average_availability: 1.0,
    availability_history: <<1::1>>,
    authorized?: false,
    authorization_date: nil,
    transport: :tcp
  ]
  • Updated the @discovery_table related MemTable. Here, we used the index to update the fields in ets tables, as we introduced a new field the new indexes got updated to relevant fields.
  • Fetching Metrics function now it is [{ip_address, http_port()}] before it is [ip_address]

Fixes #243

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

As this is related to node struct, I have updated the relevant test cases to use the new node struct as a result a lot of existing test cases were failed. So, update the relevant test units with the new node struct and see that all test cases are passed.

  • Node struct has new field http_port
  • Will have http_port part while serializing and de-serializing the node
  • Adding and connecting to node now has http_port information
  • In Metrics polling now we use the added http_port

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@ghost ghost changed the base branch from master to develop March 17, 2022 19:12
config/dev.exs Outdated Show resolved Hide resolved
test/test_helper.exs Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Mar 17, 2022

Also it would be great to add, in the metrics collector for polling, the reference to the HTTP port.

Enum.map(P2P.authorized_nodes(), & &1.ip)

@blackode blackode changed the title [WIP] Add http port in node details Add http port in node details Mar 18, 2022
@blackode blackode added P2P Involve P2P networking serialization Involve message serialization labels Mar 18, 2022
@ghost
Copy link

ghost commented Mar 18, 2022

Also it would be great to add, in the metrics collector for polling, the reference to the HTTP port.

Enum.map(P2P.authorized_nodes(), & &1.ip)

Do you need more details on this one ?

@blackode blackode requested a review from a user March 18, 2022 14:39
@ghost ghost merged commit 768a50f into develop Mar 18, 2022
@ghost ghost deleted the 243-add-http-port-node-details branch March 18, 2022 15:41
ghost pushed a commit that referenced this pull request Mar 24, 2022
@ghost ghost mentioned this pull request Apr 27, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2P Involve P2P networking serialization Involve message serialization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant