Skip to content

Conversation

@HemangChothani
Copy link
Collaborator

Towards [bigtable issue 5]

MISSING_DOCUMENT = "No document to update: "
DOCUMENT_EXISTS = "Document already exists: "
UNIQUE_RESOURCE_ID = unique_resource_id("-")
FIRESTORE_EMULATOR_HOST = "FIRESTORE_EMULATOR_HOST"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this constant. It's used once. As it's name completely repeats the value itself, there is no practical use in it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is need as chris also used here, the pattern that they follow is one is for internal variable and one is for variable which user use like here also https://github.com/q-logic/python-firestore/blob/160d7d11564a137f06faced96c21f967411bfc55/google/cloud/firestore_v1/client.py#L57

I am trying to use the same which already declared by chris.

Copy link
Collaborator

@mf2199 mf2199 left a comment

Choose a reason for hiding this comment

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

Support Ilya's comment but otherwise looks fine.

MISSING_DOCUMENT = "No document to update: "
DOCUMENT_EXISTS = "Document already exists: "
UNIQUE_RESOURCE_ID = unique_resource_id("-")
FIRESTORE_EMULATOR_HOST = "FIRESTORE_EMULATOR_HOST"
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@HemangChothani
Copy link
Collaborator Author

Added test_utils folder as it has dependency and it is already there for big table and datastore so I am not going to add into their respective PR, also i will mention in Public PR to update nox file to add session.install("-e", "test_utils") line in system test as it has dependency, nox.py file has already been updated for bigtable and datastore.

HemangChothani and others added 7 commits May 14, 2020 19:46
* feat: implement limit to last feature

* simplify test

* add more tests and de-deprecate Collection.get() method

* fix unit tests

* reverse should be used in get() only when limit_to_last set

* cosmetic changes

* use bool instead of int for _limit_to_last attr

* fix comments

* fix comments

* Apply suggestions from code review

Co-authored-by: BenWhitehead <[email protected]>

* add notes about mutually exclusivity

Co-authored-by: BenWhitehead <[email protected]>
Co-authored-by: Christopher Wilcox <[email protected]>
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.

5 participants