Add Backfilled data script #28
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alallema
force-pushed
the
backfilled-data
branch
4 times, most recently
from
e95b5d4
to
774bb13
Compare
Closed
alallema
force-pushed
the
backfilled-data
branch
6 times, most recently
from
e2f719e
to
ef9d43a
Compare
alallema
force-pushed
the
backfilled-data
branch
2 times, most recently
from
0ee91a8
to
059a2dd
Compare
alallema
force-pushed
the
backfilled-data
branch
from
059a2dd
to
73d0fea
Compare
mdubus
reviewed
Feb 15, 2022
brunoocasali
reviewed
Mar 3, 2022
| @@ -20,23 +20,20 @@ jobs: | |||
| with: | |||
| node-version: 14 | |||
| - name: Install dependencies | |||
| run: npm run install:functions | |||
| run: yarn install:functions | |||
There was a problem hiding this comment.
Any particular reason to change? 😸
There was a problem hiding this comment.
I have no particular opinion on the matter but all js repo use yarn by default so ... I change it
| - Run the project for development: | ||
| Launch Meilisearch instance: | ||
| ``` bash | ||
| curl -L https://install.meilisearch.com | sh # download Meilisearch |
|
|
||
| test('adaptDocument with fields not set and with id field in document', () => { | ||
| mockedGetFieldsToIndex.mockReturnValueOnce([]) | ||
| const snapshot = firebaseMock.firestore.makeDocumentSnapshot( |
There was a problem hiding this comment.
Could apply the http://xunitpatterns.com/Four%20Phase%20Test.html in these tests?
There was a problem hiding this comment.
I made some changes is this better?
| @@ -40,7 +40,7 @@ describe('extension', () => { | |||
|
|
|||
| beforeEach(() => { | |||
| restoreEnv = mockedEnv(defaultEnvironment) | |||
| config = require('../src/config').default | |||
| config = require('../src/config').config | |||
There was a problem hiding this comment.
This config can be loaded like this import config from '../src/config', no?
There was a problem hiding this comment.
No, defaultEnvironment variables must be loaded first otherwise the config has no access to them.
bidoubiwa
suggested changes
Mar 7, 2022
bidoubiwa
suggested changes
Mar 7, 2022
alallema
force-pushed
the
backfilled-data
branch
5 times, most recently
from
31573f4
to
9aeb93a
Compare
alallema
force-pushed
the
backfilled-data
branch
from
9aeb93a
to
ccc2e7c
Compare
bidoubiwa
suggested changes
Mar 16, 2022
| }, | ||
| "engines": { | ||
| "node": ">=14.0.0" | ||
| }, | ||
| "main": "lib/index.js", | ||
| "dependencies": { | ||
| "firebase-admin": "^9.8.0", | ||
| "meilisearch": "^0.21.0" | ||
| "firebase-functions": "^3.16.0", | ||
| "meilisearch": "^0.24.0" |
There was a problem hiding this comment.
Should be updated in a later PR
Comment on lines
+60
to
+62
| return config.meilisearch.fieldsToIndex | ||
| ? config.meilisearch.fieldsToIndex.split(/[ ,]+/) | ||
| : [] |
There was a problem hiding this comment.
Yes be before we arrive here, were they validated by your validation functions?
alallema
force-pushed
the
backfilled-data
branch
from
aaf03a0
to
5450478
Compare
alallema
force-pushed
the
backfilled-data
branch
from
5450478
to
26d094b
Compare
bidoubiwa
previously approved these changes
Mar 17, 2022
|
bors merge |
meili-bors bot
added a commit
that referenced
this pull request
Mar 17, 2022
28: Add Backfilled data script r=alallema a=alallema # Pull Request ## What does this PR do? Add a script to back-filled data from Firestore to MeiliSearch to give the possibility to import existing documents or reindex them if you already have documents in a Firestore Collection when you enable the MeiliSearch extension. Check [an example from google extension](https://github.com/firebase/extensions/tree/master/firestore-bigquery-export#backfill-your-bigquery-dataset) #15 ## PR checklist - [x] Handles different possibilities to launch the script - [x] Command line with parameter - [x] interactively - [x] Add the possibility to import documents by batch - [x] Add the possibility to import documents from sub-collections - [x] Write a guide **Notes:** - The ability to run the script with a configuration file is not part of the MVP and may be implemented later #29 Co-authored-by: alallema <[email protected]> Co-authored-by: Amélie <[email protected]>
|
Build succeeded: |
|
bors merge |
|
Already running a review |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request
What does this PR do?
Add a script to back-filled data from Firestore to MeiliSearch to give the possibility to import existing documents or reindex them if you already have documents in a Firestore Collection when you enable the MeiliSearch extension.
Check an example from google extension
#15
PR checklist
Notes: