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

Adds new v4 autocomplete search endpoint #734

Merged
merged 14 commits into from
Sep 26, 2022
Merged

Conversation

amanji
Copy link
Contributor

@amanji amanji commented Sep 21, 2022

This PR adds a new search endpoint for autocomplete searching (v4):

  • Endpoint allows to further refine autocomplete results with category filters, for example: ?category:entity_type=ULC
  • Docker compose files now pull Postgres images from docker hub.
  • Commented out db build step in manage script.

Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
docker/manage Outdated
Comment on lines 151 to 163
# build-db() {
# #
# # db
# #
# echo -e "\nBuilding vcr-postgresql image ..."
# ${S2I_EXE} build \
# --copy \
# -e "HTTP_PROXY=${HTTP_PROXY}" \
# -e "HTTPS_PROXY=${HTTPS_PROXY}" \
# '../server/db/config' \
# 'registry.access.redhat.com/rhscl/postgresql-10-rhel7:latest' \
# 'vcr-postgresql'
# }
Copy link
Member

Choose a reason for hiding this comment

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

May as well delete it.

docker/manage Outdated
@@ -229,7 +229,7 @@ build-echo-app() {

build-all() {
build-solr
build-db
# build-db
Copy link
Member

Choose a reason for hiding this comment

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

May as well delete it.

@@ -67,13 +67,16 @@ def get_remote_name(self):
def all_names(self):
return self._cached("names", self.names.all())

# DEPRECATED
Copy link
Member

Choose a reason for hiding this comment

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

Is there a better way to mark this property as deprecated, so that it will actually cause a warning when used/referenced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tbh we could just remove this too. It's not actually referenced anywhere

@ianco
Copy link
Contributor

ianco commented Sep 21, 2022

I get an error starting the services (I think due to the database change?):

vcr-agent_1   | 2022-09-21 20:24:11,389 indy.libindy.native.indystrgpostgres ERROR 	src/lib.rs:190 | Create storage failed: IOError("IO error during storage operation: database error")
vcr-agent_1   | Traceback (most recent call last):
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aries_cloudagent/indy/sdk/wallet_setup.py", line 105, in create_wallet
vcr-agent_1   |     credentials=json.dumps(self.wallet_access),
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/indy/wallet.py", line 61, in create_wallet
vcr-agent_1   |     create_wallet.cb)
vcr-agent_1   | indy.error.WalletStorageError
vcr-agent_1   | 
vcr-agent_1   | The above exception was the direct cause of the following exception:
vcr-agent_1   | 
vcr-agent_1   | Traceback (most recent call last):
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aries_cloudagent/commands/provision.py", line 36, in provision
vcr-agent_1   |     root_profile, public_did = await wallet_config(context, provision=True)
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aries_cloudagent/config/wallet.py", line 41, in wallet_config
vcr-agent_1   |     profile = await mgr.provision(context, profile_cfg)
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aries_cloudagent/indy/sdk/profile.py", line 161, in provision
vcr-agent_1   |     opened = await indy_config.create_wallet()
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aries_cloudagent/indy/sdk/wallet_setup.py", line 118, in create_wallet
vcr-agent_1   |     ) from x_indy
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aries_cloudagent/indy/sdk/wallet_setup.py", line 105, in create_wallet
vcr-agent_1   |     credentials=json.dumps(self.wallet_access),
vcr-agent_1   |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/indy/wallet.py", line 61, in create_wallet
vcr-agent_1   |     create_wallet.cb)
vcr-agent_1   | aries_cloudagent.core.error.ProfileError: Error creating wallet 'icat_agent_wallet': Error: Wallet storage error occurred
vcr-agent_1   |   Caused by: Plugin returned error

(The property names changed for the postgres user, password, etc)

Copy link
Contributor

@ianco ianco left a comment

Choose a reason for hiding this comment

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

I deleted and rebuilt the images, and I confirmed that the databases are all running against postgres:14, but the vcr-agent fails on startup when trying to create its wallet.

@amanji
Copy link
Contributor Author

amanji commented Sep 21, 2022

Did you have an instance running prior? I've noticed that if I don't tear down and remove all volumes I run into the problem of the wallet already existing.

@ianco
Copy link
Contributor

ianco commented Sep 21, 2022

Did you have an instance running prior? I've noticed that if I don't tear down and remove all volumes I run into the problem of the wallet already existing.

Yes I cleaned up all old images, volumes etc.

docker/manage Outdated
# --copy \
# -e "HTTP_PROXY=${HTTP_PROXY}" \
# -e "HTTPS_PROXY=${HTTPS_PROXY}" \
# '../server/db/config' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these configs getting applied to the new database?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They aren't, no.

@ianco
Copy link
Contributor

ianco commented Sep 21, 2022

I notice I can't login to the running database using postgres/mysecretpassword so maybe the password isn't getting applied properly?

@ianco
Copy link
Contributor

ianco commented Sep 21, 2022

Wallet db startup - you can see the failed postgres login at the end:

wallet-db_1   | syncing data to disk ... ok
wallet-db_1   | 
wallet-db_1   | 
wallet-db_1   | Success. You can now start the database server using:
wallet-db_1   | initdb: warning: enabling "trust" authentication for local connections
wallet-db_1   | You can change this by editing pg_hba.conf or using the option -A, or
wallet-db_1   | --auth-local and --auth-host, the next time you run initdb.
wallet-db_1   | 
wallet-db_1   |     pg_ctl -D /var/lib/postgresql/data -l logfile start
wallet-db_1   | 
wallet-db_1   | waiting for server to start....2022-09-21 21:14:49.546 UTC [48] LOG:  starting PostgreSQL 14.5 (Debian 14.5-1.pgdg110+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 10.2.1-6) 10.2.1 20210110, 64-bit
wallet-db_1   | 2022-09-21 21:14:49.550 UTC [48] LOG:  listening on Unix socket "/var/run/postgresql/.s.PGSQL.5432"
wallet-db_1   | 2022-09-21 21:14:49.577 UTC [49] LOG:  database system was shut down at 2022-09-21 21:14:48 UTC
wallet-db_1   | 2022-09-21 21:14:49.590 UTC [48] LOG:  database system is ready to accept connections
wallet-db_1   |  done
wallet-db_1   | server started
wallet-db_1   | CREATE DATABASE
wallet-db_1   | 
wallet-db_1   | 
wallet-db_1   | /usr/local/bin/docker-entrypoint.sh: ignoring /docker-entrypoint-initdb.d/*
wallet-db_1   | 
wallet-db_1   | waiting for server to shut down....2022-09-21 21:14:50.406 UTC [48] LOG:  received fast shutdown request
wallet-db_1   | 2022-09-21 21:14:50.415 UTC [48] LOG:  aborting any active transactions
wallet-db_1   | 2022-09-21 21:14:50.418 UTC [48] LOG:  background worker "logical replication launcher" (PID 55) exited with exit code 1
wallet-db_1   | 2022-09-21 21:14:50.431 UTC [50] LOG:  shutting down
wallet-db_1   | 2022-09-21 21:14:50.486 UTC [48] LOG:  database system is shut down
wallet-db_1   |  done
wallet-db_1   | server stopped
wallet-db_1   | 
wallet-db_1   | PostgreSQL init process complete; ready for start up.
wallet-db_1   | 
wallet-db_1   | 2022-09-21 21:14:50.558 UTC [1] LOG:  starting PostgreSQL 14.5 (Debian 14.5-1.pgdg110+1) on x86_64-pc-linux-gnu, compiled by gcc (Debian 10.2.1-6) 10.2.1 20210110, 64-bit
wallet-db_1   | 2022-09-21 21:14:50.558 UTC [1] LOG:  listening on IPv4 address "0.0.0.0", port 5432
wallet-db_1   | 2022-09-21 21:14:50.558 UTC [1] LOG:  listening on IPv6 address "::", port 5432
wallet-db_1   | 2022-09-21 21:14:50.567 UTC [1] LOG:  listening on Unix socket "/var/run/postgresql/.s.PGSQL.5432"
wallet-db_1   | 2022-09-21 21:14:50.576 UTC [62] LOG:  database system was shut down at 2022-09-21 21:14:50 UTC
wallet-db_1   | 2022-09-21 21:14:50.615 UTC [1] LOG:  database system is ready to accept connections
wallet-db_1   | 2022-09-21 21:14:57.999 UTC [69] FATAL:  password authentication failed for user "postgres"
wallet-db_1   | 2022-09-21 21:14:57.999 UTC [69] DETAIL:  Role "postgres" does not exist.
wallet-db_1   |     Connection matched pg_hba.conf line 100: "host all all all scram-sha-256"
wallet-db_1   | 2022-09-21 21:15:08.008 UTC [70] FATAL:  password authentication failed for user "postgres"
wallet-db_1   | 2022-09-21 21:15:08.008 UTC [70] DETAIL:  Role "postgres" does not exist.
wallet-db_1   |     Connection matched pg_hba.conf line 100: "host all all all scram-sha-256"

@amanji
Copy link
Contributor Author

amanji commented Sep 21, 2022

Ok I see the error now

@swcurran
Copy link
Contributor

Hey @amanji -- is one of the options for this to filter based on a topic (entity) having a specific type of credential? Not crucial, but that is a feature we've talked about in the past. If that does (or can easily) come for free with this, it would be awesome.

For example, search within all of those entities that have a Cannabis Marketing license.

Thanks! Great work regardless!

@amanji
Copy link
Contributor Author

amanji commented Sep 22, 2022

We'd need to add a filter for that but I should be able to extend the options similar to the other topic search endpoints.

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

@swcurran Filters have been added, we now should be able to filter by Issuer ID, Credential IDs held by the topic and the Type ID of the Topic itself along with Categorical fields such as entity type etc.

@swcurran
Copy link
Contributor

Way cool! @ianco -- hopefully you can get this reviewed and approved sometime soon.

@ianco
Copy link
Contributor

ianco commented Sep 23, 2022

I get these startup errors:

vcr-api_1     | ======== Running on http://0.0.0.0:8080 ========
vcr-api_1     | (Press CTRL+C to quit)
vcr-api_1     | /opt/app-root/src/agent_webhooks/utils/credential.py:680: SyntaxWarning: "is" with a literal. Did you mean "=="?
vcr-api_1     |   if model_name == "name" and (model.text is None or model.text is ""):
vcr-api_1     | /opt/app-root/src/agent_webhooks/utils/credential.py:683: SyntaxWarning: "is" with a literal. Did you mean "=="?
vcr-api_1     |   not model.type or model.value is None or model.value is ""
vcr-api_1     | >>> YES updating cred type timestamp
vcr-api_1     | >>> YES create detail claims for credentials
vcr-api_1     | /opt/app-root/src/api/v2/search/filters.py:36: SyntaxWarning: "is not" with a literal. Did you mean "!="?
vcr-api_1     |   if query_string is not "":
vcr-api_1     | /opt/app-root/src/api/v3/search_filters.py:36: SyntaxWarning: "is not" with a literal. Did you mean "!="?
vcr-api_1     |   if query_string is not "":

@ianco
Copy link
Contributor

ianco commented Sep 23, 2022

... I assume the changes to the v2 and v3 code are all just formatting right?

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

@ianco Yeah. I have the Black formatting extension in my code editor. Is there a specific style formatter we generally use?

Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

@ianco I see the same output:

But it looks to do with the webhooks code. I would suggest we do a followup PR to apply a consistent formatter to the codebase.

@ianco
Copy link
Contributor

ianco commented Sep 23, 2022

Not sure how to filter by category? I tried entity_type::BC (and other options) but didn't get any results

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

@ianco It should be like the regular search filtering so try category:entity_type=BC. I'd also make sure that the attribute types are set to 'category' if they are not 'datetime' otherwise Solr will not index topic_category properly.

@ianco
Copy link
Contributor

ianco commented Sep 23, 2022

The v4 endpoints are complicated to use. The /topic/{id} requires a numerical id but the autocomplete doesn't return this value. Previews api versions (e.g. v3 take the source_id as a parameter rather than the numerical id, I think this makes more sense for the api (end users are more likely to know the source id rather than the orgbook internal numerical id).

If we don't want to change this endpoint then we should include the id in the autocomplete search results.

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

I can add id into the results. Is there anything else we should include?

@ianco
Copy link
Contributor

ianco commented Sep 23, 2022

try category:entity_type=BC.

Tried this it didn't have any affect on the search outputs (entity type BC vs SP returned the same list of corps)

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

What do your Topic indexes look like in Solr?

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

Screen.Recording.2022-09-23.at.10.44.03.AM.mov

Here's an example of the search filter in action. Not your Topic indexes should look like:

{
        "id":"api_v2.topic.3",
        "django_ct":"api_v2.topic",
        "django_id":"3",
        "topic_source_id":"BC0014947",
        "topic_issuer_id":2,
        "topic_type_id":2,
        "topic_inactive":false,
        "topic_revoked":false,
        "topic_name":["BC0014947",
          "PATTISON SECURITIES LIMITED"],
        vvv
        "topic_category":["registration_id::BC0014947",
          "entity_name::PATTISON SECURITIES LIMITED",
          "entity_status::ACT",
          "entity_type::BC",
          "home_jurisdiction::BC",
          "reason_description::Event:CONVICORP"],
        ^^^
        "topic_credential_type_id":[2],
        "topic_all_credentials_inactive":false,
        "topic_all_credentials_revoked":false,
        "_version_":1744782618584416256
}

@amanji
Copy link
Contributor Author

amanji commented Sep 23, 2022

Screen.Recording.2022-09-23.at.2.37.37.PM.mov

An example of entity_type filters being applied along with a name suggestion directly on topic. This could obviate the need to have names and addresses indexed since the information is available directly in the Topic index.

Signed-off-by: Akiff Manji <akiff.manji@gmail.com>
@ianco
Copy link
Contributor

ianco commented Sep 26, 2022

Inconsistent results between the v2 and v4 (and v3) autocomplete. Searching for inc the v2 results include:

{
  "total": 89,
  "page_size": 10,
  "page": 1,
  "first_index": 1,
  "last_index": 10,
  ... etc
}

The v4 (and v3) results include:

{
  "total": 10,
  "first_index": 1,
  "last_index": 10,
  ... etc
}

None of the api's include params to adjust the page size or offset, but the v2 is the only one that includes the overall record count.

@ianco
Copy link
Contributor

ianco commented Sep 26, 2022

try category:entity_type=BC.

Tried this it didn't have any affect on the search outputs (entity type BC vs SP returned the same list of corps)

Tried again and one of the returned corps was A0064760. Doing a search on this corp the entity_type was A so it looks like the filter isn't working.

As an aside the comment on this param in the swagger page is Filter by Credential Category. The category name and value should be joined by '::' which is incorrect.

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.

None yet

4 participants