-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Filters should support properties with leading/trailing whitespaces in the name. #424
Comments
I'm happy to support this but it seems like allowing developers to define properties with any spaces is asking for undefined or unexpected behavior. Adding leading and trailing spaces even more so. ISTM there are two separate points / sides to take:
In my mind, the 2nd point is the more valid one, but we are not at all being clear about this in our development. In addition we are spending time making I strongly prefer a future that involves running Luckily, if we do decide to implement this @Alfus comments in #294 carve out a good chunk of what needs to be done. |
I is possible to have it both ways with two different versions of the but there really should be a way to specify a filter on any supported On Tue Dec 16 2014 at 8:13:10 PM Danny Hermes [email protected]
|
I really don't understand the use case for the two-argument form:
versus the more natural / idomatic three-argument spelling:
As a bonus, the three-argument form makes the problem with leading / trailing (even embedded) spaces go away. |
I'm for the three-argument form as well. @jgeewax? #294 (comment) |
FYI to all. I am working on this right now and changing the signature to def filter(self, property_name, operation, value): so that now
will be the way to call This makes it much simpler to support property names with any arbitrary kind of whitespace and also is a true mapping to the underlying proto (I mentioned this before). In light of my comment above, being a truer mapping is a good thing. Please chime in if there are any issues or concerns. |
Also adding a test for property names with spaces in them. Fixes googleapis#424.
Also adding a test for property names with spaces in them. Fixes googleapis#424.
Also adding a test for property names with spaces in them. Fixes googleapis#424.
Also adding a test for property names with spaces in them. Fixes googleapis#424.
…424) Source-Link: googleapis/synthtool@8e55b32 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:c6c965a4bf40c19011b11f87dbc801a66d3a23fbc6704102be064ef31c51f1c3
…424) Source-Link: googleapis/synthtool@8e55b32 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:c6c965a4bf40c19011b11f87dbc801a66d3a23fbc6704102be064ef31c51f1c3
* fix(deps): Require google-api-core >=1.34.0, >=2.11.0 fix: Drop usage of pkg_resources fix: Fix timeout default values docs(samples): Snippetgen should call await on the operation coroutine before calling result PiperOrigin-RevId: 493260409 Source-Link: googleapis/googleapis@fea4387 Source-Link: googleapis/googleapis-gen@387b734 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMzg3YjczNDRjNzUyOWVlNDRiZTg0ZTYxM2IxOWE4MjA1MDhjNjEyYiJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * add gapic_version.py * fix(deps): require google-api-core>=1.34.0,>=2.11.0 * chore: use templated setup.py and owlbot.py * update version in gapic_version.py * work around generator bug Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Anthonios Partheniou <[email protected]>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
* chore: Update gapic-generator-python to v1.11.2 PiperOrigin-RevId: 546510849 Source-Link: googleapis/googleapis@736073a Source-Link: googleapis/googleapis-gen@deb64e8 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZGViNjRlOGVjMTlkMTQxZTMxMDg5ZmU5MzJiM2E5OTdhZDU0MWM0ZCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Co-authored-by: Anthonios Partheniou <[email protected]>
Source-Link: https://togithub.com/googleapis/synthtool/commit/30bd01b4ab78bf1b2a425816e15b3e7e090993dd Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:9bc5fa3b62b091f60614c08a7fb4fd1d3e1678e326f34dd66ce1eefb5dc3267b
) Source-Link: https://togithub.com/googleapis/synthtool/commit/d6103f4a3540ba60f633a9e25c37ec5fe7e6286d Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:39f0f3f2be02ef036e297e376fe3b6256775576da8a6ccb1d5eeb80f4c8bf8fb
…424) Source-Link: googleapis/synthtool@8e55b32 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:c6c965a4bf40c19011b11f87dbc801a66d3a23fbc6704102be064ef31c51f1c3 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
…[autoapprove] (#424) Source-Link: googleapis/synthtool@4826337 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:60a63eddf86c87395b4bb394fdddfe30f84a7726ee8fe0b758ea132c2106ac75 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
…424) Source-Link: googleapis/synthtool@8e55b32 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-python:latest@sha256:c6c965a4bf40c19011b11f87dbc801a66d3a23fbc6704102be064ef31c51f1c3
.filter('property with space =', 1)
or.filter(' property with leading and trailing spaces =', 1)
should result in the properties names not losing the trailing/leading whitespaces.More context in #294 (comment)
The text was updated successfully, but these errors were encountered: