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

Change the warnings section of the status to be full error object #1497

Closed
amorton opened this issue Oct 4, 2024 · 3 comments · Fixed by #1519
Closed

Change the warnings section of the status to be full error object #1497

amorton opened this issue Oct 4, 2024 · 3 comments · Fixed by #1519
Assignees

Comments

@amorton
Copy link
Contributor

amorton commented Oct 4, 2024

Currently it looks like this:

{
    "data": {
        "documents": [
            {
                "partition": "partition_1",
                "value": 2,
                "key": 2
            },
            {
                "partition": "demo_1",
                "value": 2,
                "key": 2
            }
        ],
        "nextPageState": null
    },
    "status": {
        "warnings": [
            "The filter includes columns that are not indexed. \n\nThe table demo.partitioned_tinyint_key has the primary key: partition(text), key(tinyint).\nAnd has indexes on the columns: [None].\nThe request filtered on the un-indexed columns: value.\n\nThe query was executed without taking advantage of the primary key or indexes on the table, this can have performance implications on large tables.\n\nSee documentation at XXXX for best practices for filtering."
        ]
    }
}

The warnings we make for allow filtering in the code use the ApiException, and in writing the code it would have been easier to just see the ID for the error we created (for the warning).,

Change the warnings list to be a list of ApiExceptions and return the fill error object V2 structure in the response so it has family, scope etc.

change so in the warnings, it is a list of error object v2 like below:

{
            "message": "Only columns defined in the table schema can be filtered on.\n\nThe table demo.partitioned_tinyint_key defines the columns: partition(text), key(tinyint), value(int).\nThe filter included the following unknown columns: valueq.\n\nResend the request using only defined columns.",
            "scope": "FILTER",
            "errorCode": "UNKNOWN_TABLE_COLUMNS",
            "id": "c0c68d62-615d-4650-8275-b87264bed1c8",
            "family": "REQUEST",
            "title": "Unknown table columns in filter"
        }
@amorton amorton self-assigned this Oct 7, 2024
@amorton
Copy link
Contributor Author

amorton commented Oct 7, 2024

starting :)

@amorton
Copy link
Contributor Author

amorton commented Oct 14, 2024

Almost ready, adding an example here to flag to client and downstream peeps.

So given this input...

{
    "createNamespace": {
        "name": "stargate",
        "options": {
            "replication": {
                "class": "SimpleStrategy",
                "replication_factor": 2
            }
        }
    }
}

The response from the API has changed to be :

{
    "status": {
        "warnings": [
            {
                "errorCode": "DEPRECATED_COMMAND",
                "message": "A deprecated command was used, it may still be used but will be removed in future releases.\n\nThe deprecated command is: createNamespace.\nThe new command to use is: createKeyspace.\n\nPlease check the documentation for the new command and update your code.",
                "family": "REQUEST",
                "scope": "WARNING",
                "title": "Deprecated command",
                "id": "c887e159-2f05-4da5-95a1-5f53b32c852b"
            }
        ],
        "ok": 1
    }
}

Previously, after the work in #1406 the the warnings section of the status was an array of strings, not a full error object. Moving to the error object means we can template the message, and clients can / should code against the familty / scope / code for the error not the message.

@tatu-at-datastax
Copy link
Contributor

Would it make sense to use scope for same usage as with actual Errors, to indicate affected area? For example, when requiring ALLOW FILTERING, scope could be... query/schema/index (whatever most accurate scope we have is).
"WARNING" does not seem like a scope, but more like a type.
(also leading to the question of what kind of Error is Warning? :-) )

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 a pull request may close this issue.

2 participants