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

refactor: refactor firestore into package with pytyped for type checkers #448

Merged
merged 2 commits into from
Oct 13, 2021

Conversation

nipunn1313
Copy link
Contributor

@nipunn1313 nipunn1313 commented Sep 17, 2021

Wasn't sure how to test this within the testsuite. I tested manually with

python3 -m venv venv
source venv/bin/activate
pip install -e ~/src/python-firestore
mypy --python-executable=venv/bin/python test.py

with test.py

from google.cloud.firestore import AsyncClient
from google.cloud.firestore_v1 import AsyncClient

line 1 fails on main, but passes with this change

(venv) ➜  src mypy --python-executable=venv/bin/python test.py
test.py:2: error: Skipping analyzing "google.cloud.firestore": found module but no type hints or library stubs
test.py:2: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

vs

(venv) ➜  src mypy --python-executable=venv/bin/python test.py
Success: no issues found in 1 source file

Fixes #447 🦕

@nipunn1313 nipunn1313 requested a review from a team as a code owner September 17, 2021 22:31
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Sep 17, 2021
@product-auto-label product-auto-label bot added the api: firestore Issues related to the googleapis/python-firestore API. label Sep 17, 2021
Refactor firestore.py into firestore package with py.typed
@tseaver tseaver added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Sep 21, 2021
@crwilcox crwilcox added the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 12, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Oct 12, 2021
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 12, 2021
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 12, 2021
@crwilcox crwilcox changed the title Refactor firestore into package with pytyped refactor: refactor firestore into package with pytyped for type checkers Oct 12, 2021
@crwilcox
Copy link
Contributor

I verified that a small script using sync and async clients runs successfully post change

import asyncio
import os
from google.cloud import firestore

print("SYNC CLIENT")

db = firestore.Client()

# Add a new document
print("Creating document")
doc_ref = db.collection(u'users').document(u'alovelace')
doc_ref.set({  # Crash
    u'first': u'Ada',
    u'last': u'Lovelace',
    u'born': 1815
})

print("ASYNC CLIENT")

async def async_test():
    db = firestore.AsyncClient()

    # Add a new document
    print("Creating document")
    doc_ref = db.collection(u'users').document(u'alovelace')
    await doc_ref.set({  # Crash
        u'first': u'Ada',
        u'last': u'Lovelace',
        u'born': 1815
    })

loop = asyncio.get_event_loop()
coroutine = async_test()
loop.run_until_complete(coroutine)

@crwilcox crwilcox removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 13, 2021
@crwilcox crwilcox merged commit 4fd4923 into googleapis:main Oct 13, 2021
@crwilcox crwilcox mentioned this pull request Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/python-firestore API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

google.cloud.firestore lacks packaged typing
4 participants