[Issue #1697] add more foreign table models and update setup_foreign_tables.py #1769
Conversation
| @@ -55,19 +55,21 @@ def copy_oracle_data(db_session: db.Session) -> None: | |||
|
|
|||
| try: | |||
| with db_session.begin(): | |||
| _run_copy_commands(db_session, Schemas.API) | |||
| _run_copy_commands(db_session, Schemas.API, Schemas.FOREIGN) | |||
There was a problem hiding this comment.
I think we want to make a new script - we don't want to break the v0 opportunity setup.
jamesbursa
requested review from
acouch,
aplybeah and
SammySteiner
as code owners
| from . import foreignbase | ||
|
|
||
|
|
||
| class Tforecast(foreignbase.ForeignBase): |
There was a problem hiding this comment.
Should we name the classes Foreign... otherwise if we follow this pattern for the staging tables, we'll have two sets of classes with identical names which might be confusing / get imported incorrectly. I can also just aim to name the staging tables as StagingTopportunity and we leave these as-is.
There was a problem hiding this comment.
Depending how the staging code works, could it be useful to have the names match? Could imagine something like:
from src.db import foreign, staging
tables = (foreign.Tforecast, foreign.TforecastHist, ...) # all 18 tables
for foreign_table in tables:
staging_table = getattr(staging, foreign_table.__name__)
do_staging_copy(foreign_table, staging_table, ...)There was a problem hiding this comment.
That's fair - might make configuring / maintaining the copy portion of the process simpler - just need to match up the modules.
| class ForeignTableDDLCompiler(sqlalchemy.sql.compiler.DDLCompiler): | ||
| """SQLAlchemy compiler for creating foreign tables.""" | ||
|
|
||
| def create_table_constraints(self, _table, _include_foreign_key_constraints=None, **kw): |
There was a problem hiding this comment.
Missing type definitions on a few of these. FIne to just set them to Any if they're complex internal types to SQLAlchemy and not just str/bool or similar. A quick check of the base class, it doesn't look to type them inline, so not a lot to go on.
Summary
Fixes #1697
Time to review: 10 mins
Changes proposed
src.db.foreignmodule defining the foreign tables as SQLAlchemy models.setup_foreign_tables.pyto use the new models instead of its own definition.copy_oracle_data.pyto use the new database schema.Context for reviewers
Currently we only have one table from the source database available as a foreign table. This change adds the remaining tables, and also moves them into a separate schema.
Additional information