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

feat: Do not query /jobs/triggers with a partialFilter #1571

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

paultranvan
Copy link
Contributor

One might want to avoid querying the /jobs/triggers route and stick to the /data route, to avoid extra work from the stack, that can be costful.
We were using the existence of worker and type keys in selector, but a partialFilter existence should also force the use of the /data route.

One might want to avoid querying the `/jobs/triggers` route and stick to
the `/data` route, to avoid extra work from the stack, that can be
costful.
We were using the existence of `worker` and `type` keys in selector, but
a partialFilter existence should also force the use of the `/data`
route.
@@ -122,7 +122,8 @@ class TriggerCollection extends DocumentCollection {
*/
async find(selector = {}, options = {}) {
const { worker, type, ...rest } = selector
const hasOnlyWorkerAndType = Object.keys(rest).length === 0
const hasOnlyWorkerAndType =
Object.keys(rest).length === 0 && !options.partialFilter
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: hasOnlyWorkerAndType is not accurate anymore

Copy link
Contributor Author

@paultranvan paultranvan Dec 10, 2024

Choose a reason for hiding this comment

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

Hmm I don't think so, hasOnlyWorkerAndType is true if there is no other keys than worker and type in selector and NO partialFilter.
Reversely, if there is another key or a partialFilter, hasOnlyWorkerAndType is false

Copy link
Contributor

@zatteo zatteo Dec 10, 2024

Choose a reason for hiding this comment

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

and NO partialFilter that part made me think that it is not accurate anymore.

Is it possible that there is no worker and type in selector but a partial filter is present ?

edit: oh they are linked together, so it can not happen ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible that there is no worker and type in selector but a partial filter is present ?

Yes it can. And in this case, hasOnlyWorkerAndType is false, as there is a partialFilter

@paultranvan paultranvan merged commit bb380fa into master Dec 10, 2024
3 checks passed
@paultranvan paultranvan deleted the trigger-find branch December 10, 2024 16:12
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.

2 participants