Skip to content

Conversation

@erezrokah
Copy link
Member

@erezrokah erezrokah commented Apr 13, 2023

Copy link
Member Author

@erezrokah erezrokah Apr 13, 2023

Choose a reason for hiding this comment

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

@bubbajoe
Copy link
Contributor

bubbajoe commented Apr 13, 2023

Thanks, this already looks really well done! Can you add docs for the following fields?

MaxBatchSize - maximum batch size for each request. context: when testing extracting millions of rows. If the request took longer than a minute the batch would timeout (at around 400-500k rows in my case). So I set the default to 50,000 IIRC. This might need to be configured depending on row data size + bandwidth + cpu, etc..

OrderByField - this is optional but you can get your results ordering by a specific fields (yes, this can be more than 1, maybe I should have made it plural: OrderByFields). This requires you to add an index, which can only be done on the Firebase UI right now. https://pkg.go.dev/cloud.google.com/go/firestore#Query.OrderBy

OrderByDirection - ASC or DESC. Not used unless the above is field is specified but I think this is self explanatory.

@bubbajoe
Copy link
Contributor

@erezrokah Forgot to tag you here ^

@erezrokah erezrokah force-pushed the feat/firestore_website branch from 721f21e to 7f7925f Compare April 16, 2023 12:35
@erezrokah erezrokah requested review from a team and candiduslynx and removed request for a team April 16, 2023 12:35
@erezrokah
Copy link
Member Author

Thanks @bubbajoe, that's super helpful. I followed up with 7f7925f and renamed those to order_by and order_direction. Do that make sense?

Also it seems the spec values were not used so I fixed that, see 7f7925f#diff-2900389b353248fc48ac304a58d98219fa4c209abf43ccb456a43f4952db22f5R63

Please let me know if that looks OK.

@bubbajoe
Copy link
Contributor

Thanks! Looks really good!

@erezrokah erezrokah requested review from disq and removed request for candiduslynx April 16, 2023 12:41
@erezrokah erezrokah added the automerge Automatically merge once required checks pass label Apr 16, 2023
@kodiakhq kodiakhq bot merged commit 9586efc into cloudquery:main Apr 16, 2023
kodiakhq bot pushed a commit that referenced this pull request Apr 17, 2023
🤖 I have created a release *beep* *boop*
---


## 1.0.0 (2023-04-17)


### Features

* **firestore:** Add firestore source ([#9653](#9653)) ([0fe7a6c](0fe7a6c))
* **firestore:** Add Sentry ([#10003](#10003)) ([aad54bc](aad54bc))
* **firestore:** Upgrade to `github.com/cloudquery/plugin-sdk/v2` ([#9956](#9956)) ([44ae1ba](44ae1ba)), closes [#9954](#9954)
* **website:** Add Firestore ([#10007](#10007)) ([9586efc](9586efc))


### Bug Fixes

* **deps:** Update module github.com/cloudquery/plugin-sdk/v2 to v2.2.0 ([#10135](#10135)) ([cf33b89](cf33b89))
* **firestore:** Change default version to 'development' ([#10005](#10005)) ([8527b66](8527b66))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@erezrokah
Copy link
Member Author

@erezrokah erezrokah deleted the feat/firestore_website branch April 17, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants