fix(modelql)!: disable implicit queries to avoid accidental performan… #1234
Conversation
…ce issues You should fix the unintended implicit queries or use .query explicitly. You can also set the environment variable MODELQL_IMPLICIT_QUERIES_ENABLED=true to re-enable this feature.
slisson
force-pushed
the
MODELIX-649-Consequences-of-on-demand-mode-via-model-api-instances-after-ModelQl-query-hard-to-understand-for-users
branch
from
57e46ee to
6ba247c
Compare
…ce issues You should fix the unintended implicit queries or use .query explicitly. You can also set the environment variable MODELQL_IMPLICIT_QUERIES_ENABLED=true to re-enable this feature.
slisson
force-pushed
the
MODELIX-649-Consequences-of-on-demand-mode-via-model-api-instances-after-ModelQl-query-hard-to-understand-for-users
branch
from
6ba247c to
b6cd91a
Compare
JVM coverage report
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| package org.modelix.modelql.client | ||
|
|
||
| actual val implicitQueriesEnabled: Boolean = System.getenv("MODELQL_IMPLICIT_QUERIES_ENABLED")?.toBooleanStrictOrNull() ?: false |
Check warning
Code scanning / detekt
The property implicitQueriesEnabled is missing documentation. Warning
There was a problem hiding this comment.
Wouldn't it be a better idea to make the choice if implicit queries are allowed or not a constructor or builder argument of the client? That way, application can decide on a suitable configuration mechanism for them to apply.
Otherwise, the new option would need to be explained in the docs.
This would also allow clients to be configured differently in one application. |
|
I was thinking about removing this feature completely, but didn't want to make switching to a new version unnecessarily difficult. A poor performance is still better than exceptions. Still not sure wether we should support this feature or deprecate it. |
Pull request was closed
No description provided.