[AGE-383] Refactor Postgres/PostGIS import - #83
Conversation
| @@ -1,43 +1,46 @@ | |||
| from scrapy.spiders import SitemapSpider | |||
| from scrapy.crawler import CrawlerProcess | |||
There was a problem hiding this comment.
the only change in this is to use constants, the rest is ruff reformatting
| ) # Default model | ||
|
|
||
| MIN_CHUNK_TOKENS = 10 # anything less provides no value | ||
| MAX_CHUNK_TOKENS = 8191 |
There was a problem hiding this comment.
i did up this from 7000 to match the actual max from OPENAI
There was a problem hiding this comment.
I wonder why 7000 was chosen? Might be some inconsistency in how tokens are counted by the client vs api? But 1191 is a huge margin.
I think this is the right call, just need to keep an eye out for problems.
| @@ -0,0 +1,26 @@ | |||
| import os | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
these were defined all of the place, consolidated
| from psycopg.sql import SQL, Identifier | ||
|
|
||
|
|
||
| class DocumentImporter(ABC): |
There was a problem hiding this comment.
postgres & postgres imports are so similar, created a base class to create a base structure
murrayju
left a comment
murrayju
left a comment
There was a problem hiding this comment.
Only lightly skimmed, but lgtm 👍
| ) # Default model | ||
|
|
||
| MIN_CHUNK_TOKENS = 10 # anything less provides no value | ||
| MAX_CHUNK_TOKENS = 8191 |
There was a problem hiding this comment.
I wonder why 7000 was chosen? Might be some inconsistency in how tokens are counted by the client vs api? But 1191 is a huge margin.
I think this is the right call, just need to keep an eye out for problems.
2 participants
What
Refactor
postgres_docs.pyandpostgis_docs.pyso we can share functionality between the two.Other changes:
Why
This is a follow up to a external PR submission -- decision was to merge that PR then address refactoring.
Testing
I tested postgres and postgis locally and verified that the outputs look good. I scanned through all of the chunks and spot checked a bunch of them.
I have attached the outputs from both
postgres.json
postgis.json
Related PR
#59