Use new GRPC hub API if it's available #3314

jeffreypicard · 2021-05-31T22:08:25Z

To run the go hub tests.
Clone and run the hub from here https://github.com/lbryio/hub using branch "claim_search_port"

git clone git@github.com:lbryio/hub.git
./dev.sh
Alternatively you can build the hub and copy the lbry/wallet/bin in this repo.
git clone git@github.com:lbryio/hub.git
go build .
cp hub <lbry-sdk-location>/lbry/wallet/bin/
Run the tests
export GO_HUB=true
python -m unittest discover tests.integration.hub.test_hub_commands
The claim search tests now also all work with GO_HUB=true set.
python -m unittest discover tests.integration.blockchain.test_claim_commands

You can also set HUB_HOST and HUB_PORT, it defaults to localhost:50051

eukreign · 2021-06-01T17:43:36Z

should this be marked as Draft?

jeffreypicard · 2021-06-14T15:40:22Z

Actually, I think the hub needs to be merged and create a release first, so the HubNode can download the latest release from there if needed.

eukreign · 2021-06-14T15:45:19Z

all of the automated tests should be green before this is moved out of Draft

eukreign

Is test_hub_commands.py testing something that isn't tested by test_claim_commands.py? Do we need both?

See inline comments for more feedback.

lbry/wallet/network.py

lbry/wallet/orchstr8/node.py

jeffreypicard · 2021-06-14T23:47:12Z

I fixed the issues mentioned and the build works on ubuntu, both the windows and macos build fail so I commented them out for now so that I could make sure the ubuntu build would actually run and pass.

lyoshenka · 2021-06-15T19:52:24Z

does this connect the new hub code to our CI/CD process?

jeffreypicard · 2021-06-15T20:12:47Z

does this connect the new hub code to our CI/CD process?

I set the environment variable GO_HUB=true in the github pipeline yaml, so as long as that works the way I think it does, it should be using my go hub for the tests.

lyoshenka · 2021-06-23T15:33:09Z

to add:

drop env var and always have it enabled
drop HUB_HOST - it should use the same host as the Python hub
drop HUB_PORT and hardcode it to 50051
when connecting to the Python hub, also try to connect to the GRPC port 50051. if it connection works, then use that for claim_search. if not, fall back to using the existing python claim_search
- hook into existing reconnect event to do this
- do this on the first claim_search for each connection. this avoids flooding the server

lyoshenka · 2021-07-12T20:29:20Z

todo: review the kwargs changes

some of the changes remove features (like allowing claim_ids and not_claim_ids at the same time)
need to have consistency between the SDK api fields and the hub api fields

Have the basic starting /stopping / querying. Still don't have the hub jsonrpc stuff working right and from the looks of it I need to clearify some of the logic in the claim search function itself because it's not returning the correct number of claims anyways. get the integration working with grpcurl Got tests working, still need to port the rest of them ported all of the claim search tests still a few failing due to not having inflation working, and there's something weird with limit_claims_per_channel that needs to be fixed.

Fixing tests relabel failing tests properly run all the tests for the hub cleanup HubNode

more protobuf changes (fix imports)

Cleanup prints and commented out code remove print don't do list claims cleanup

correctly.

lyoshenka · 2021-08-09T20:27:17Z

lbry/extras/daemon/daemon.py

+        except Exception as e:
+            if self.ledger.config['use_go_hub']:
+                log.warning("failed, trying again without hub")
+                self.ledger.config['use_go_hub'] = False
+                return await self.jsonrpc_claim_search(**kwargs_old)


@jeffreypicard two things

we shouldn't modify the config like this. if another config is saved elsewhere, this setting will also be saved

if this fallback is disabled, a bunch of tests fail. therefore we think this is hiding test failures by falling back to old working code.

more generally, if tests pass for all the new code, then errors here should be treated like production errors -- the sdk should report the error to the user.

lyoshenka · 2021-09-08T15:41:17Z

closing because #3394 supersedes it

lbry-bot assigned eukreign May 31, 2021

eukreign assigned jeffreypicard and unassigned eukreign Jun 1, 2021

jeffreypicard changed the title ~~Golang claim search tests~~ WIP: Golang claim search tests Jun 1, 2021

jeffreypicard marked this pull request as draft June 2, 2021 14:00

jeffreypicard changed the title ~~WIP: Golang claim search tests~~ Golang claim search tests Jun 14, 2021

jeffreypicard marked this pull request as ready for review June 14, 2021 15:16

jeffreypicard requested a review from eukreign June 14, 2021 15:33

lbry-bot assigned eukreign Jun 14, 2021

jeffreypicard requested a review from shyba June 14, 2021 15:33

lbry-bot assigned shyba Jun 14, 2021

eukreign marked this pull request as draft June 14, 2021 15:45

eukreign requested changes Jun 14, 2021

View reviewed changes

lbry-bot unassigned eukreign and jeffreypicard Jun 14, 2021

lyoshenka marked this pull request as ready for review June 14, 2021 20:25

lyoshenka mentioned this pull request Jun 15, 2021

Do CI/CD for new hub #3290

Open

lyoshenka mentioned this pull request Jun 15, 2021

Integrate new hub into integration tests and hub docker-compose.yml #3289

Open

jeffreypicard force-pushed the golang_claim_search_tests branch from cf59d37 to a927cac Compare June 15, 2021 20:07

jeffreypicard force-pushed the golang_claim_search_tests branch 3 times, most recently from 619ab7a to ae21fc3 Compare June 17, 2021 17:45

lyoshenka changed the title ~~Golang claim search tests~~ Use new GRPC hub API if it's available Jun 23, 2021

jeffreypicard force-pushed the golang_claim_search_tests branch from 557c5da to 78fc9a3 Compare June 23, 2021 21:29

jeffreypicard force-pushed the golang_claim_search_tests branch 3 times, most recently from 78fc9a3 to ce87c37 Compare July 22, 2021 19:47

jeffreypicard added 13 commits July 23, 2021 11:57

grpc client for python

7e6895b

cleanup and reorgnazing some stuff

4381675

Fixing tests relabel failing tests properly run all the tests for the hub cleanup HubNode

protobuf changes

7803e1d

more protobuf changes (fix imports)

uncomment tests, add remove_duplicates param

a406da0

Cleanup prints and commented out code remove print don't do list claims cleanup

update protobufs

d2144ee

tons of small changes squashed together

5495757

updates protobuf

43f6423

remove uneeded prints

b672c9b

Forgot to remove duplicate tests

7ecaf8b

Set default server to the networks default and use go hub by default

eeef0d5

Move the go hub settings from network to ledger config and hook reset

b1ec63a

correctly.

hardcode port

e21c9e8

jeffreypicard force-pushed the golang_claim_search_tests branch 2 times, most recently from 11f9817 to e21c9e8 Compare July 23, 2021 17:54

jeffreypicard added 2 commits July 23, 2021 14:48

update code to be consistent with field renames

5796bba

updates for fields

da778f8

lyoshenka reviewed Aug 9, 2021

View reviewed changes

shyba mentioned this pull request Aug 11, 2021

Golang claim search rebased and fixes #3394

Merged

lyoshenka closed this Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use new GRPC hub API if it's available #3314

Use new GRPC hub API if it's available #3314

jeffreypicard commented May 31, 2021 •

edited

Loading

eukreign commented Jun 1, 2021

jeffreypicard commented Jun 14, 2021

eukreign commented Jun 14, 2021

eukreign left a comment

jeffreypicard commented Jun 14, 2021

lyoshenka commented Jun 15, 2021

jeffreypicard commented Jun 15, 2021

lyoshenka commented Jun 23, 2021 •

edited

Loading

lyoshenka commented Jul 12, 2021

lyoshenka Aug 9, 2021

lyoshenka commented Sep 8, 2021

Use new GRPC hub API if it's available #3314

Use new GRPC hub API if it's available #3314

Conversation

jeffreypicard commented May 31, 2021 • edited Loading

eukreign commented Jun 1, 2021

jeffreypicard commented Jun 14, 2021

eukreign commented Jun 14, 2021

eukreign left a comment

Choose a reason for hiding this comment

jeffreypicard commented Jun 14, 2021

lyoshenka commented Jun 15, 2021

jeffreypicard commented Jun 15, 2021

lyoshenka commented Jun 23, 2021 • edited Loading

lyoshenka commented Jul 12, 2021

lyoshenka Aug 9, 2021

Choose a reason for hiding this comment

lyoshenka commented Sep 8, 2021

jeffreypicard commented May 31, 2021 •

edited

Loading

lyoshenka commented Jun 23, 2021 •

edited

Loading