search

octabytes / FireO

Google Cloud Firestore modern and simplest convenient ORM package in Python. FireO is specifically designed for the Google's Firestore
https://fireo.octabyte.io
Apache License 2.0
249 stars 29 forks source link

use filter key for query to fix warning #224

Closed jessicamann closed 4 months ago

jessicamann commented 4 months ago

positional arguments for 'where' clause is deprecated and results in a warning. the new syntax is documented here: 'https://cloud.google.com/firestore/docs/query-data/queries#python_1'

This address Issue #220

jessicamann commented 4 months ago

@AxeemHaider could you help me with the next steps on getting this merged in if it's good?

ADR-007 commented 4 months ago

Hi @jessicamann! Thank you for your PR! I may suggest you to make sure that the version of "google-cloud-firestore" specified in setup.py is still correct and update it if it is not true install_requires=['google-cloud-firestore==2.11.1,<=2.12.0'],

jessicamann commented 4 months ago

Hi @jessicamann! Thank you for your PR! I may suggest you to make sure that the version of "google-cloud-firestore" specified in setup.py is still correct and update it if it is not true install_requires=['google-cloud-firestore==2.11.1,<=2.12.0'],

Ah, got it, I'll double check.

jessicamann commented 4 months ago

@ADR-007 the existing version constraint works with the change.

jessicamann commented 4 months ago

I'm not able to run the test suite that comes with the repo, however (I don't have a separate GCP project that I can use for testing). Would you be able to help with running that?

I just ran this change by linking the updated package code to my own repo that is using the fireo dependency.

markConklin commented 4 months ago

+1, thank you

jessicamann commented 4 months ago

@ADR-007 thank you for the approval!

It looks like only maintainers can merge the pull request. Let me know if there's anything else I can do to help this along. Thanks for taking a look 🙏🏻

markConklin commented 4 months ago

any chance we can get this merged in? I would love to clean up my logs and reduce some noice.

ADR-007 commented 4 months ago

@jessicamann

I just fixed the CI. Could you please pull latest master and push it here, so I can re-run CI? Thanks

jessicamann commented 4 months ago

@ADR-007 just pulled the latest from master!

ADR-007 commented 4 months ago

Hi @AxeemHaider, could you please take a look on this:

Error: google-github-actions/auth failed with: the GitHub Action workflow must specify exactly one of "workload_identity_provider" or "credentials_json"! If you are specifying input values via GitHub secrets, ensure the secret is being injected into the environment. By default, secrets are not passed to workflows triggered from forks, including Dependabot.

markConklin commented 4 months ago

@AxeemHaider thanks so much for merging this is in! what are the next steps for cutting a new release?

AxeemHaider commented 4 months ago

@markConklin it's available you can update to version 2.2.2

AxeemHaider commented 4 months ago

Hi @AxeemHaider, could you please take a look on this:

Error: google-github-actions/auth failed with: the GitHub Action workflow must specify exactly one of "workload_identity_provider" or "credentials_json"! If you are specifying input values via GitHub secrets, ensure the secret is being injected into the environment. By default, secrets are not passed to workflows triggered from forks, including Dependabot.

@ADR-007 The authentication failure issue is caused by restrictions from GitHub, which prevent the use of GCP resources for pull requests. If we remove this security measure, automatic tests will run on every pull request. This could allow malicious users to submit resource-intensive pull requests, leading to increased GCP charges.

However, we can mitigate this risk by modifying our workflow: tests should only run on pull requests that have been approved by a reviewer. This way, we ensure that only legitimate changes trigger the tests. Currently, I am unsure how to implement this change, but it seems like a viable solution.