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

Do not clutter the logs with warnings in prod environment #1109

Closed
vicb opened this issue May 4, 2023 · 7 comments
Closed

Do not clutter the logs with warnings in prod environment #1109

vicb opened this issue May 4, 2023 · 7 comments
Assignees
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@vicb
Copy link
Contributor

vicb commented May 4, 2023

The following code

 const companyQuery = query
   .filter('name', 'Google')
   .filter('size', '<', 400);

will print a warning:

process.emitWarning(
  'Providing Filter objects like Composite Filter or Property Filter is recommended when using .filter'
);

First I don't think there should be any warning in a prod environment.
One way js libraries check for prod environemnt is to check if the NODE_ENV environment variable is set to production - see the express docs for ref.

Also my understanding is that the warning is there because of nested composite filters. I don't think there should be any warnings in a simple case, that is:

const companyQuery = query.filter('name', 'Google');

should probably not generate a warning even in dev env.

/cc @danieljbruce

@vicb vicb added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels May 4, 2023
@product-auto-label product-auto-label bot added the api: datastore Issues related to the googleapis/nodejs-datastore API. label May 4, 2023
@danieljbruce
Copy link
Contributor

Datastore in Python emits this warning too so whatever we end up doing there should probably be consistency across the board.

@vicb
Copy link
Contributor Author

vicb commented May 4, 2023

I am not familiar with Python warnings but it looks like you can at least filter the warnings

@danieljbruce danieljbruce self-assigned this May 5, 2023
@vicb
Copy link
Contributor Author

vicb commented Jun 25, 2023

This is what I mean:

image

It would be great to filter out all the noise in my prod env.

Thanks!

@MiguiTE
Copy link

MiguiTE commented Jun 29, 2023

What should be done not to get this warning when building a query with filters?

According to docs there are two ways of filtering a query, with operator and without (defaults to equals). So, how could we build and supply a Filter object?

@vicb
Copy link
Contributor Author

vicb commented Jun 29, 2023

It looks like the doc is not up to date. The filter method can now take objects. You can look at the tests for an example.

That being said I think using the legacy way (with and without operator) should still be supported and should not log a warning at least in production mode.

@efossvold
Copy link

Using PropertyFilter eliminates the warning.

import { Datastore, PropertyFilter } from '@google-cloud/datastore'
const ds = new Datastore()
const query = ds.createQuery('Kind')
.filter(new PropertyFilter('foo', '=', 'bar'))
.filter(new PropertyFilter('baz', '=', 'quux'))

@vicb
Copy link
Contributor Author

vicb commented Jul 5, 2023

@efossvold read my initial message. I said I don't think we should be forced to do that for simple case because there is no point. Thanks for trying to help though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the googleapis/nodejs-datastore API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants