Light miner: Diagnostics page should NOT display blockchain details #142

pritamghanghas · 2022-03-25T15:23:38Z

grpc client for communication with gatewayrs

Link:
Summary: Ground work to make the diagnostic change required by this ticket

How
Write a small client for local.proto grpc service.
Try and maintain compatibility with old interface.
Keep it separate from jsonrpc code even if this means a little bit of copied code.

Screenshots

References

Checklist

Tests added
Cleaned up commit history (rebase!)
Documentation added
Thought about variable and method names

pritamghanghas · 2022-03-25T15:31:41Z

hm_pyhelper/gateway_grpc/client.py

+LOGGER = get_logger(__name__)
+
+
+def decode_pub_key(encoded_key: bytes) -> str:


this functions is as is copy from jsonrpc

Shouldn't the old files be deleted as they will no longer be usable?

I was hoping that we will be able to merge this into master and not change all requirements.txt to point to a git branch

removed json_rpc files.

pritamghanghas · 2022-03-25T15:33:10Z

hm_pyhelper/gateway_grpc/client.py

+        return json.loads(response.decode())
+
+
+def get_address_from_add_gateway_txn(add_gateway_txn:


this functions is also as is copy from jsonrpc client

pritamghanghas

code smells are in grpc generated code. Will have to ignore.

pritamghanghas · 2022-03-25T15:35:22Z

requirements.txt

+protobuf==3.19.3
+grpcio==1.44.0
+grpcio-tools==1.44.0


I don't think grpcio-tools is required for runtime but needed for generating the code, we don't have a dev requirements.txt

Feel free to make dev-requirements

added. Assuming only additional ones have to go in there just like test-requirements

pritamghanghas · 2022-03-28T13:54:08Z

hm_pyhelper/gateway_grpc/client.py

+        return self.get_validator_info().height
+
+    def get_region_enum(self) -> int:
+        '''


Letting possible grpc exceptions pass through for all grpc invocations. Caller might be in a better position.
Catching them here and raising MinerFetchException might be good abstraction but I didn't see any value in that.

marvinmarnold · 2022-03-28T19:42:11Z

protos/update_protos.sh

@@ -0,0 +1,26 @@
+#!/bin/bash


This file should be mentioned in the README

hm_pyhelper/gateway_grpc/client.py

marvinmarnold · 2022-03-28T19:45:58Z

hm_pyhelper/gateway_grpc/client.py

+LOGGER = get_logger(__name__)
+
+
+def decode_pub_key(encoded_key: bytes) -> str:


Shouldn't the old files be deleted as they will no longer be usable?

marvinmarnold · 2022-03-28T19:51:00Z

hm_pyhelper/gateway_grpc/client.py

+            for line in output.decode().splitlines():
+                if line.strip().startswith('Version'):
+                    return line.split(':')[1].strip()
+            return 'unknown'


Can we return None or raise an exception instead?

returned None

marvinmarnold · 2022-03-28T19:53:29Z

protos/update_protos.sh

+update_proto https://raw.githubusercontent.com/helium/proto/master/src/service/local.proto local.proto
+update_proto https://raw.githubusercontent.com/helium/proto/master/src/region.proto region.proto
+
+# python -m grpc_tools.protoc -I=$SRC_DIR --python_out=$DST_DIR $SRC_DIR/blockchain_txn.proto


Avoid committing unused code

marvinmarnold · 2022-03-28T19:56:33Z

protos/update_protos.sh

@@ -0,0 +1,26 @@
+#!/bin/bash
+
+# NOTE:: It would be better if we just pull in the helium proto repo as a submodule.


This is a weird place to make a comment like this. I also don't agree with the statement.

our protos are out of sync because nothing keeps them updated. A submodule will show such variance with simple git commands. If I download the latest blockchain_txn.proto all these files will be missing:

import "blockchain_txn_coinbase_v1.proto"; import "blockchain_txn_security_coinbase_v1.proto"; import "blockchain_txn_oui_v1.proto"; import "blockchain_txn_gen_gateway_v1.proto"; import "blockchain_txn_routing_v1.proto"; import "blockchain_txn_payment_v1.proto"; import "blockchain_txn_security_exchange_v1.proto"; import "blockchain_txn_consensus_group_v1.proto"; import "blockchain_txn_consensus_group_failure_v1.proto"; import "blockchain_txn_add_gateway_v1.proto"; import "blockchain_txn_assert_location_v1.proto"; import "blockchain_txn_create_htlc_v1.proto"; import "blockchain_txn_redeem_htlc_v1.proto"; import "blockchain_txn_poc_request_v1.proto"; import "blockchain_txn_poc_receipts_v1.proto"; import "blockchain_txn_vars_v1.proto"; import "blockchain_txn_rewards_v1.proto"; import "blockchain_txn_token_burn_v1.proto"; import "blockchain_txn_dc_coinbase_v1.proto"; import "blockchain_txn_token_burn_exchange_rate_v1.proto"; import "blockchain_txn_state_channel_open_v1.proto"; import "blockchain_txn_update_gateway_oui_v1.proto"; import "blockchain_txn_state_channel_close_v1.proto"; import "blockchain_txn_payment_v2.proto"; import "blockchain_txn_price_oracle_v1.proto"; import "blockchain_txn_gen_price_oracle_v1.proto"; import "blockchain_txn_transfer_hotspot_v1.proto"; import "blockchain_txn_rewards_v2.proto"; import "blockchain_txn_gen_validator_v1.proto"; import "blockchain_txn_stake_validator_v1.proto"; import "blockchain_txn_transfer_validator_stake_v1.proto"; import "blockchain_txn_unstake_validator_v1.proto"; import "blockchain_txn_validator_heartbeat_v1.proto"; import "blockchain_txn_assert_location_v2.proto"; import "blockchain_txn_transfer_hotspot_v2.proto";

The other options is to evaluated whether we even need this proto.

resolved during daily standup. Manual copying continues.

marvinmarnold · 2022-03-28T19:58:15Z

hm_pyhelper/tests/test_gateway_grpc.py

+
+    def test_client(self):
+        with GatewayClient(f'localhost:{TestData.server_port}') as client:
+            self.assertEqual(client.get_pubkey(), TestData.pubkey_decoded)


Better to make each of these different tests

Done. Lot of repetition though

…ails (#321) grpc client for communication with gatewayrs

sonarqubecloud · 2022-03-29T14:46:24Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
17 Code Smells

No Coverage information
0.0% Duplication

pritamghanghas · 2022-03-29T14:48:51Z

hm_pyhelper/miner_param.py

@@ -182,13 +180,10 @@ def get_ethernet_addresses(diagnostics):
    for (path, key) in zip(path_to_files, keys):
        try:
            diagnostics[key] = get_mac_address(path)
-        except MinerFailedToFetchMacAddress as e:


seemed like dead useless code.

pritamghanghas · 2022-03-29T14:49:20Z

hm_pyhelper/miner_param.py

@@ -206,8 +201,6 @@ def get_mac_address(path):
             The path must be a string value")
    try:
        file = open(path)
-    except MinerFailedToFetchMacAddress as e:


same here. why would open throw one of our exceptions.

marvinmarnold · 2022-03-29T19:23:43Z

hm_pyhelper/miner_param.py

@@ -182,13 +180,10 @@ def get_ethernet_addresses(diagnostics):
    for (path, key) in zip(path_to_files, keys):
        try:
            diagnostics[key] = get_mac_address(path)
-        except MinerFailedToFetchMacAddress as e:


marvinmarnold · 2022-03-29T19:27:39Z

hm_pyhelper/miner_param.py

@@ -206,8 +201,6 @@ def get_mac_address(path):
             The path must be a string value")
    try:
        file = open(path)
-    except MinerFailedToFetchMacAddress as e:


pritamghanghas requested a review from a team as a code owner March 25, 2022 15:23

pritamghanghas commented Mar 25, 2022

View reviewed changes

pritamghanghas force-pushed the gateway-grpc branch from c766c97 to 368ecba Compare March 25, 2022 15:34

pritamghanghas commented Mar 25, 2022

View reviewed changes

pritamghanghas mentioned this pull request Mar 27, 2022

Light miner: Remove dbus container and upnp NebraLtd/hm-config#181

Merged

4 tasks

MuratUrsavas approved these changes Mar 28, 2022

View reviewed changes

pritamghanghas force-pushed the gateway-grpc branch from 368ecba to 97c4c46 Compare March 28, 2022 11:30

pritamghanghas commented Mar 28, 2022

View reviewed changes

pritamghanghas force-pushed the gateway-grpc branch from 97c4c46 to add87e3 Compare March 28, 2022 14:23

marvinmarnold reviewed Mar 28, 2022

View reviewed changes

pritamghanghas self-assigned this Mar 29, 2022

pritamghanghas force-pushed the gateway-grpc branch 4 times, most recently from 43c8b04 to f578d90 Compare March 29, 2022 13:56

feat: Light miner: Diagnostics page should NOT display blockchain det…

4a492c2

…ails (#321) grpc client for communication with gatewayrs

pritamghanghas force-pushed the gateway-grpc branch from f578d90 to 4a492c2 Compare March 29, 2022 14:41

pritamghanghas commented Mar 29, 2022

View reviewed changes

marvinmarnold approved these changes Mar 29, 2022

View reviewed changes

pritamghanghas merged commit 228892c into light-hotspot Mar 30, 2022

shawaj deleted the gateway-grpc branch August 28, 2022 13:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Light miner: Diagnostics page should NOT display blockchain details #142

Light miner: Diagnostics page should NOT display blockchain details #142

pritamghanghas commented Mar 25, 2022 •

edited by shawaj

Loading

pritamghanghas Mar 25, 2022 •

edited

Loading

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

pritamghanghas Mar 29, 2022

pritamghanghas Mar 25, 2022

pritamghanghas left a comment

pritamghanghas Mar 25, 2022

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

pritamghanghas Mar 28, 2022 •

edited

Loading

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

marvinmarnold Mar 28, 2022

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

pritamghanghas Mar 29, 2022 •

edited

Loading

pritamghanghas Mar 29, 2022

marvinmarnold Mar 28, 2022

pritamghanghas Mar 29, 2022

sonarqubecloud bot commented Mar 29, 2022

pritamghanghas Mar 29, 2022

marvinmarnold Mar 29, 2022

pritamghanghas Mar 29, 2022 •

edited

Loading

marvinmarnold Mar 29, 2022

marvinmarnold Mar 29, 2022

marvinmarnold Mar 29, 2022

		LOGGER = get_logger(__name__)


		def decode_pub_key(encoded_key: bytes) -> str:

		return json.loads(response.decode())


		def get_address_from_add_gateway_txn(add_gateway_txn:

		@@ -0,0 +1,26 @@
		#!/bin/bash

		# NOTE:: It would be better if we just pull in the helium proto repo as a submodule.

Light miner: Diagnostics page should NOT display blockchain details #142

Light miner: Diagnostics page should NOT display blockchain details #142

Conversation

pritamghanghas commented Mar 25, 2022 • edited by shawaj Loading

pritamghanghas Mar 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pritamghanghas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pritamghanghas Mar 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pritamghanghas Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pritamghanghas Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pritamghanghas commented Mar 25, 2022 •

edited by shawaj

Loading

pritamghanghas Mar 25, 2022 •

edited

Loading

pritamghanghas Mar 28, 2022 •

edited

Loading

pritamghanghas Mar 29, 2022 •

edited

Loading

pritamghanghas Mar 29, 2022 •

edited

Loading