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

Transition service groups to a persistent data store (comparable to the service endpoints store) #481

Merged
merged 5 commits into from
Apr 10, 2024

Conversation

cmickeyb
Copy link
Contributor

Service group files loaded on demand has been one of the more painful complexities for running contracts. This commit introduces a persistent database comparable to the database used to store service endpoints. The new groups database is stored at the location specified in the ['Service', 'GroupDatabaseFile'] configuration entry.

The service groups commands were all updated to accommodate the new style. These are SIGNIFICANT changes. To ensure that correctness has not been compromised, a new test was added for storage groups.

Finally, the persistent database required updating several other tests. This change is, for the moment, a "functionally correct" change to the tests (meaning that all tests now use the correct service groups commands) but there is significant room for cleaning up the process.

Note that this will require changes to the test scripts for PDO contracts (and the notebooks that set up the service groups). The correct approach would be (for tests) to invoke the site-configuration.psh script in place of the create-service-groups script. And a future jupyter notebook will create an interface for managing the database more explicitly.

@cmickeyb cmickeyb added the client Issues related to building, configuring and running a client label Mar 21, 2024
@cmickeyb cmickeyb force-pushed the mic.mar20.group_db branch 2 times, most recently from fe74f29 to e8edb1e Compare March 27, 2024 14:55
@cmickeyb cmickeyb force-pushed the mic.mar20.group_db branch from e8edb1e to 9e83acf Compare April 3, 2024 15:57
Copy link
Contributor

@g2flyer g2flyer left a comment

Choose a reason for hiding this comment

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

Seems a useful addition to simplify stuff and works with any noticeable problems when i test. However, find a number observations/suggestions as comments

client/pdo/client/commands/service_groups.py Show resolved Hide resolved
client/docs/service_groups.md Show resolved Hide resolved
client/docs/service_groups.md Show resolved Hide resolved
client/docs/service_groups.md Show resolved Hide resolved
client/docs/service_groups.md Outdated Show resolved Hide resolved
build/tests/service-groups-test.psh Show resolved Hide resolved
client/docs/service_groups.md Show resolved Hide resolved
build/tests/shell-test.psh Show resolved Hide resolved
client/pdo/client/commands/pservice.py Show resolved Hide resolved
Copy link
Member

@bvavala bvavala left a comment

Choose a reason for hiding this comment

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

No particular comments about the PR itself, but more a high-level question.

While client, eservice and pservice are core components of PDO, the service group concept appears to be a useful tool to put things together and create interesting tests and use cases. The question is: are service groups the way PDO will be deployed and used, or is it just one integration approach?

client/docs/service_groups.md Outdated Show resolved Hide resolved
@@ -102,28 +102,77 @@ for v in $(seq 0 $((${port_count} - 1))) ; do
done

# -----------------------------------------------------------------
say run unit tests for eservice database
# these functions attemp to create a reproducible environment for
Copy link
Contributor

Choose a reason for hiding this comment

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

pdo test files are now very complex. For someone new looking to understand PDO infra, a logical starting might be to see the test files to understand what can be done with PDO (disregarding the contracts repo); however, the service-test.sh file that we have now compared to run-tests.sh that we had a couple of years ago is too complex to understand without digging deeper into several of the features that are being used as part of the test itself. not related directly to this pr, but the pr adds to the complexity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is no way we could have even begun to test pdo with run-tests. its still there, by the way (see service-test.sh). sorry... this is where i say, tests are tough and not the way to learn the system.

client/docs/service_groups.md Outdated Show resolved Hide resolved
client/docs/service_groups.md Outdated Show resolved Hide resolved

try pdo-eservice create --group test1
try pdo-eservice add --group test1 --url http://${F_SERVICE_HOST}:7101 http://${F_SERVICE_HOST}:7102
try pdo-eservice add --group test1 --name es7103 es7104
Copy link
Contributor

Choose a reason for hiding this comment

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

in line 168, you are assuming that there is only one eservice-db available per environment, and you are also assuming its name ? May be pdo-eservice add --group test1 --db-name --name es7103 es7104 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can specify group database and service database on the command line.

@cmickeyb
Copy link
Contributor Author

cmickeyb commented Apr 5, 2024

No particular comments about the PR itself, but more a high-level question.

While client, eservice and pservice are core components of PDO, the service group concept appears to be a useful tool to put things together and create interesting tests and use cases. The question is: are service groups the way PDO will be deployed and used, or is it just one integration approach?

groups are useful for clients not just tests. they bundle a bunch of configuration that would otherwise have to be specified independently. they are strictly a client tool. and... anyone can replace our current client libraries with their own, if they choose. if you completely replaced the python client with a javascript one, would keep groups? maybe.

Copy link
Contributor

@prakashngit prakashngit left a comment

Choose a reason for hiding this comment

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

thank you for the PR.

@g2flyer g2flyer self-requested a review April 8, 2024 17:10
@cmickeyb cmickeyb force-pushed the mic.mar20.group_db branch from 2048946 to 25e08ff Compare April 8, 2024 17:31
cmickeyb and others added 5 commits April 9, 2024 17:00
Co-authored-by: Bruno Vavala <[email protected]>
Co-authored-by: Michael Steiner <[email protected]>
Signed-off-by: Mic Bowman <[email protected]>
Service group files loaded on demand has been one of the more painful
complexities for running contracts. This commit introduces a
persistent database comparable to the database used to store service
endpoints. The new groups database is stored at the location specified
in the ['Service', 'GroupDatabaseFile'] configuration entry.

The service groups commands were all updated to accommodate the new
style. These are SIGNIFICANT changes. To ensure that correctness has
not been compromised, a new test was added for storage groups.

Finally, the persistent database required updating several other
tests.  This change is, for the moment, a "functionally correct"
change to the tests (meaning that all tests now use the correct
service groups commands) but there is significant room for cleaning up
the process.

Signed-off-by: Mic Bowman <[email protected]>
The pdo-create-service-groups script was necessary to
load service groups on client startup. That functionality
can now be used far more flexibly with pdo-service-groups and
the pdo-[eps]service commands.

Signed-off-by: Mic Bowman <[email protected]>
Site.psh was another means of configuring the services and
service groups. That functionality for users can be more easily
provided by pdo-service-groups and pdo-[eps]service commands.

However, we still need the ability to reset the service and
groups databases for running tests. So we moved site.psh to
the tests directory and incorporated it into test execution.

Signed-off-by: Mic Bowman <[email protected]>
Moved the check for URL existence into the actual store operation
so it should be applied for all adds. there is a new verify()
operation on groups that can check to make sure the URLs still exist
in the db, but not goint to go any further for enforcing data
integrity at this time.

Signed-off-by: Mic Bowman <[email protected]>
@cmickeyb cmickeyb force-pushed the mic.mar20.group_db branch from 3e24641 to b516c5f Compare April 9, 2024 23:03
@prakashngit prakashngit merged commit 6f699de into hyperledger-labs:main Apr 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Issues related to building, configuring and running a client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants