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

Replace the eservice json database with a more functional database that can be used for all service types #447

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

cmickeyb
Copy link
Contributor

Replace the eservice database with more general service database

The eservice database was limited by what it could store (just enclave services), how it was stored (full file load/save), and how the information was exported.

The old eservice database is replaced by a persistent database (implemented with lmdb) that stores information about all services. The database is loaded on first use (similar to the block store manager) from settings in the configuration file.

The eservice_db pdo-shell command is replaced with one for all service databases (using the new format).

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.

Nice generalization that takes advantage of lmdb (a dependency that pdo already has).
I don't have comments.

The eservice database was limited by what it could store (just enclave
services), how it was stored (full file load/save), and how the
information was exported.

The old eservice database is replaced by a persistent database
(implemented with lmdb) that stores information about all
services. The database is loaded on first use (similar to the block
store manager) from settings in the configuration file.

Signed-off-by: Mic Bowman <[email protected]>
Move file handling for import/export of service group
and service data into the shell commands leaving the
group and data code independent of the ultimate file
format (shell commands use TOML with configuration
expansions).

Update shell test to include the common "service_host"
binding variable that we use throughout other tests.

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

Looks good, leaving a few questions mostly for clarification. Thank you.

return serialized

def verify(self, ledger_config = None) :
"""ensure that the eservice still exists and hosts the enclave, and
Copy link
Contributor

Choose a reason for hiding this comment

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

i thought in the past, we always verified an entry with ledger before adding to the db ? now it seems that you expect the client to explicitly verify an entry with the ledger after adding to db? (for example, line 523 where you store by url, you are simply calling fetch, but no verify). I am not sure why client would want to add anything to db without verifying.

build/tests/service-test.sh Show resolved Hide resolved
Copy link
Member

@marcelamelara marcelamelara left a comment

Choose a reason for hiding this comment

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

This is a super useful update, thank you!

@prakashngit prakashngit merged commit 6308d9c into hyperledger-labs:main Sep 14, 2023
4 checks passed
@cmickeyb cmickeyb deleted the mic.aug22.service_data branch January 16, 2024 00:21
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.

4 participants