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

[INDY-1237] Get rid of RegistryPoolManager #595

Merged

Conversation

ArtObr
Copy link
Contributor

@ArtObr ArtObr commented Mar 28, 2018

Signed-off-by: ArtObr [email protected]

@@ -95,14 +94,9 @@ def get_name_by_rank(self, rank, nodeReg=None) -> Optional[str]:
class HasPoolManager:
# noinspection PyUnresolvedReferences, PyTypeChecker
def __init__(self, nodeRegistry=None, ha=None, cliname=None, cliha=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need nodeRegistry func param?

@ArtObr
Copy link
Contributor Author

ArtObr commented Apr 6, 2018

Test this please

@@ -28,7 +28,8 @@ def cli2(cliLooper, tdir, tdirWithClientPoolTxns,
cli.close()


def testEachClientOnDifferentPort(cli1, cli2):
@pytest.mark.skip(reason='node registry removal')
def testEachClientOnDifferentPort(cli1, cli2, txnPoolNodeSet):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these tests can be removed

@@ -32,6 +32,7 @@


# noinspection PyIncorrectDocstring
@pytest.mark.skip(reason='get rid of registry pool')
def testClientShouldNotBeAbleToConnectToNodesNodeStack(pool):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this test can be removed

@@ -54,6 +55,7 @@ def testClientShouldNotBeAbleToConnectToNodesNodeStack(pool):


# noinspection PyIncorrectDocstring
@pytest.mark.skip(reason='get rid of registry pool')
def testSendRequestWithoutSignatureFails(pool):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need a similar test based on SDK to check that we handle missing signatures properly


def test_req_id_key_error(testNode, wallet1):
@pytest.mark.skip(reason='test changed in INDY-1029')
def test_req_id_key_error(looper, txnPoolNodeSetNotStarted, wallet1):
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any sense in this test...

@@ -105,6 +109,7 @@ def testPluginManagerSendMessageUponSuspiciousSpike(
assert sent == 3


@pytest.mark.skip(reason='get rid of registry pool')
Copy link
Contributor

Choose a reason for hiding this comment

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

We can not skip this test, it's needed.

@@ -126,6 +131,7 @@ def mockProcessRequest(obj, inc=1):
assert sent == 3


@pytest.mark.skip(reason='get rid of registry pool')
Copy link
Contributor

Choose a reason for hiding this comment

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

We can not skip this test, it's needed.



# noinspection PyIncorrectDocstring
@pytest.mark.skip(reason='get rid of registry pool')
def testConnectWithoutKeySharingFails(tdir_for_func, tconf_for_func):
Copy link
Contributor

Choose a reason for hiding this comment

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

We can not skip this test, it's needed.

@ashcherbakov ashcherbakov merged commit 0a996ba into hyperledger:master Apr 17, 2018
lovesh pushed a commit to evernym/indy-plenum that referenced this pull request May 3, 2018
* Get rid of RegistryPoolManager

Signed-off-by: ArtObr <[email protected]>

* Removal of nodeRegistry param

Signed-off-by: ArtObr <[email protected]>

* fixes

Signed-off-by: ArtObr <[email protected]>

* fix

Signed-off-by: ArtObr <[email protected]>
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