Add a default server config for non-source nodes.#2035
Conversation
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
jlhester
force-pushed
the
1913-default-server
branch
from
697af5b to
9a48408
Compare
jlhester
force-pushed
the
1913-default-server
branch
2 times, most recently
from
e869335 to
2db1392
Compare
| return self.registry.catalogs.get(self.deployment_spec.default_catalog) | ||
| settings = get_settings() | ||
| if settings.seed_setup.default_catalog_name: | ||
| return self.registry.catalogs.get(settings.seed_setup.default_catalog_name) |
There was a problem hiding this comment.
Should we make sure that the registry always loads up the default catalog in the catalogs bulk load?
There was a problem hiding this comment.
When you say "loads up", do you mean actually create the default catalog if it doesn't exist, or just check if it exists during the bulk load?
There was a problem hiding this comment.
Oh, I meant just check if it exists during bulk load. I think an explicit creation step is probably cleaner than just auto-creating.
| if catalogs: | ||
| new_revision.catalog_id = catalogs[0] | ||
| else: | ||
| default_catalog = await Catalog.get_default_catalog(session) |
There was a problem hiding this comment.
Should this use the same pattern as above (lines 542-547), i.e., "use the default catalog or fallback to virtual catalog"?
jlhester
force-pushed
the
1913-default-server
branch
from
ca2e9a3 to
3e93226
Compare
jlhester
force-pushed
the
1913-default-server
branch
from
3e93226 to
2b47c33
Compare
|
One of the tests failed initially with a sqlalchemy distinct key conflict, but rerunning fixed it 😬. If it pops up again I'll capture it in a ticket. |
2 participants
Summary
Test Plan
make checkpassesmake testshows 100% unit test coverageDeployment Plan