-
Notifications
You must be signed in to change notification settings - Fork 42
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
Transition service groups to a persistent data store (comparable to the service endpoints store) #481
Conversation
fe74f29
to
e8edb1e
Compare
e8edb1e
to
9e83acf
Compare
There was a problem hiding this 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
There was a problem hiding this 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?
@@ -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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
||
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 |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
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. |
There was a problem hiding this 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.
2048946
to
25e08ff
Compare
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]>
3e24641
to
b516c5f
Compare
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.