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

Query failure when server spec is missing x-maturity #572

Closed
tokebe opened this issue Mar 3, 2023 · 6 comments
Closed

Query failure when server spec is missing x-maturity #572

tokebe opened this issue Mar 3, 2023 · 6 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers

Comments

@tokebe
Copy link
Member

tokebe commented Mar 3, 2023

@colleenXu found an issue when writing test SmartAPI specifications. If the x-maturity property is missing from a server entry, the following occurs within the logs, with all queries failing:

{
    "timestamp": "2023-03-03T07:48:59.339Z",
    "level": "DEBUG",
    "message": "call-apis: 2 planned queries for edge undefined",
    "code": null
},
{
    "timestamp": "2023-03-03T07:48:59.340Z",
    "level": "ERROR",
    "message": "TypeError: Cannot read properties of undefined (reading 'endsWith') while configuring query. Query dump: [object Object]",
    "code": null
},

Additionally, a quick change for this error text would be good. the Query dump should be the JSON of the query, rather than just [object Object]. This can be accomplished by changing the relevant log from query.toString() to JSON.stringify(query).

@colleenXu, if you could provide a test yaml to replicate this issue with, that would be much appreciated.

@tokebe tokebe added bug Something isn't working enhancement New feature or request good first issue Good for newcomers labels Mar 3, 2023
@rjawesome
Copy link
Contributor

I also saw this issue as well.

@rjawesome rjawesome self-assigned this Mar 3, 2023
@colleenXu
Copy link
Collaborator

colleenXu commented Mar 3, 2023

so will_error_mychem.txt is a copy of mychem on biolink 3.1.1, but with the x-maturity property removed from the servers objects.

normal servers object:

servers:
- description: Encrypted Production server
  url: https://mychem.info/v1
  x-maturity: production
- description: Production server
  url: http://mychem.info/v1
  x-maturity: production

with x-maturity removed:

servers:
- description: Encrypted Production server
  url: https://mychem.info/v1
- description: Production server
  url: http://mychem.info/v1

The steps are:

  • download the yaml file, change the extension to .yaml
  • set BTE up to use this local file (smartapi override), run the smartapi sync with override settings
  • run BTE. send a query through the v1/smartapi/{id}/query endpoint (one is included below)
A test query for MyChem

{
    "message": {
        "query_graph": {
            "nodes": {
                "n0": {
                    "categories": ["biolink:SmallMolecule"],
                    "ids": ["CHEMBL.COMPOUND:CHEMBL440"]
                },
                "n1": {
                    "categories": ["biolink:Disease"]
                }
            },
            "edges": {
                "e01": {
                    "subject": "n0",
                    "object": "n1"
                }
            }
        }
    }
}
Relevant console logs
  bte:biothings-explorer-trapi:batch_edge_query Start to query APIEdges.... +0ms
  bte:call-apis:query Resolving ID feature is turned on +0ms
  bte:call-apis:query Number of API Edges received is 7 +1ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query using template builder +0ms
  bte:call-apis:query Query configuration error, query skipped +1ms
  bte:call-apis:query Query configuration error, query skipped +0ms
  bte:call-apis:query Query configuration error, query skipped +0ms
  bte:call-apis:query Query configuration error, query skipped +0ms
  bte:call-apis:query Query configuration error, query skipped +0ms
  bte:call-apis:query Query configuration error, query skipped +0ms
  bte:call-apis:query Query configuration error, query skipped +1ms
  bte:call-apis:query query completes. +0ms
  bte:call-apis:query Total number of records returned for this query is 0 +0ms

@colleenXu
Copy link
Collaborator

colleenXu commented Mar 3, 2023

Original questions:

  • Is this behavior intended, where the x-maturity property is required for BTE to ingest a SmartAPI yaml correctly? It seems like it'd be helpful if it wasn't (like we could provide "production" as a default).
  • Would it be helpful to catch an error like the server's url being undefined?

It's not clear right now if x-maturity should be expected as a required property for servers objects...

Rest of the original Slack message for this issue

Low-priority question: I'm testing BTE with local SmartAPI yaml files (new files with x-bte info), and I came across unexpected behavior.

The unexpected behavior:

If the SmartAPI yaml's servers section objects are missing the x-maturity property, BTE doesn't seem to ingest the server url correctly (APIEdge.query_operation.server variable ends up undefined).

Example servers section of a SmartAPI yaml:

servers:
  - description: Production server
    url: 'https://api.pharmgkb.org/v1'

BTE then doesn't raise any errors. However, all queries will fail because the subqueries will fail. The relevant console/TRAPI logs are kind of confusing. I've pasted the TRAPI logs below:

        {
            "timestamp": "2023-03-03T07:48:59.339Z",
            "level": "DEBUG",
            "message": "call-apis: 2 planned queries for edge undefined",
            "code": null
        },
        {
            "timestamp": "2023-03-03T07:48:59.340Z",
            "level": "ERROR",
            "message": "TypeError: Cannot read properties of undefined (reading 'endsWith') while configuring query. Query dump: [object Object]",
            "code": null
        },

@tokebe
Copy link
Member Author

tokebe commented Mar 3, 2023

Adding a requirement that it must be ensured that BTE can fully handle cases of specs without an x-maturity. The work to cover this issue should cover that requirement, but additional testing will be required using the INSTANCE_ENV environment variable to ensure that such specs are handled properly in all deployment situations.

@rjawesome
Copy link
Contributor

See pull requests

@tokebe
Copy link
Member Author

tokebe commented Mar 28, 2023

Deployed to prod 🚀

@tokebe tokebe closed this as completed Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants