Change the public schema name with ENV vars#445

pavlovdog · 2025-02-03T12:39:21Z

No description provided.

DZakh

Looks good. I left one comment regarding tests. Also, I remember there were some queries in the Rust code. Could you double check them as well 👌

DZakh · 2025-02-03T12:54:24Z


 describe("Entity History Codegen", () => {
  it("Creates a postgres insert function", () => {
-    let expected = `CREATE OR REPLACE FUNCTION "insert_TestEntity_history"(history_row "public"."TestEntity_history", should_copy_current_entity BOOLEAN)


Let's keep the expected as a plain string

I agree here, the tests should not have dynamic environment values for assertions

DZakh

Looks really good 👍 This is an amazing work and cool to see little improvements not related to the main problem. I left one comment regarding using EnvState in db.rs. Also, you're our first external contributor, so I need to share our Contributor License Agreement (CLA) with you, since we created it after you've started working on the PR 😅

I asked @JonoPrest to make a final review. I'll merge it and create a release after he approves 💪

JonoPrest

Hey @pavlovdog, thanks so much for the contribution 🔥

Before merging we just need to sanitise the the schema name on raw query strings to handle mixed casing. And it would be good to see a test that uses an alt schema name to validate everything works as expected. Perhaps it's just worth duplicating one of the integration tests where an alterenative schema name is set before start up.

JonoPrest

Approving since my comments are not blocking for merge, it's already an improvement 👍🏼

This reverts commit 1b3918d.

Co-authored-by: Jono Prest <65739024+JonoPrest@users.noreply.github.com>

pavlovdog · 2025-02-11T18:15:44Z

Hey @JonoPrest, thanks so much for your review! I'm happy to be your first contributor :) I've updated the code with your comments, test_codegen is passing.

I'm happy to cover this with a test, do you think I can copy-paste integration-raw-events-test.ts with public. replaced with custom.? I guess I would need to put it into a separate folder, like scenarios/test_custom_pg_schema, and add it into the CI so it is executed with ENVIO_PG_PUBLIC_SCHEMA=custom. What do you think?

JonoPrest · 2025-02-12T09:45:22Z

Hey @JonoPrest, thanks so much for your review! I'm happy to be your first contributor :) I've updated the code with your comments, test_codegen is passing.

I'm happy to cover this with a test, do you think I can copy-paste integration-raw-events-test.ts with public. replaced with custom.? I guess I would need to put it into a separate folder, like scenarios/test_custom_pg_schema, and add it into the CI so it is executed with ENVIO_PG_PUBLIC_SCHEMA=custom. What do you think?

Awesome, I think it can be as simple as duplicating this test in the integration test script

hyperindex/codegenerator/integration_tests/tests/runAll.sh

Lines 31 to 33 in 1eaf596

    
           export TEMPLATE="test_indexers/dynamic_contracts" 
        
           export TEST_FILE="DynamicContracts" 
        
           CONFIG_FILE="config-dynamic-contracts.yaml" SHOULD_FAIL=false TEST_RESTART=true ./tests/testIndexerExits.sh

with a schema set in the env. Lets do it in a separate PR

DZakh reviewed Feb 3, 2025

View reviewed changes

pavlovdog force-pushed the main branch 3 times, most recently from 70bcb85 to e83badf Compare February 5, 2025 12:26

DZakh reviewed Feb 10, 2025

View reviewed changes

Comment thread codegenerator/cli/src/persisted_state/db.rs Outdated

Comment thread codegenerator/cli/templates/static/codegen/src/db/Migrations.res

DZakh requested a review from JonoPrest February 10, 2025 11:22