Add differing capacities

[ericcccsliu]

Pull Request

Description

Renames the capacity_kw column to inverter_capacity_kw, and adds a module_capacity_kw column to store the nameplate capacity of the solar panels on the site.

How Has This Been Tested?

• [x] Adjusted conftest.py to populate fake sites with both inverter and module capacities
• [x] Ran tests using make test

Checklist:

• [x] My code follows OCF's coding style guidelines
• [x] I have performed a self-review of my own code
• [x] I have made corresponding changes to the documentation
• [x] I have added tests that prove my fix is effective or that my feature works
• [x] I have checked my code and corrected any misspellings

Todos:

• Corresponding changes will have to be made in pv-site-api, and potentially in other repositories, to account for the re-naming and the additional column for each site.

[codecov[bot]]

Codecov Report

Merging #69 (e54e4b9) into main (11080b3) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #69      +/-   ##
==========================================
+ Coverage   93.17%   93.23%   +0.06%     
==========================================
  Files           9        9              
  Lines         205      207       +2     
==========================================
+ Hits          191      193       +2     
  Misses         14       14              

Impacted Files	Coverage Δ
pvsite_datamodel/sqlmodels.py	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ericcccsliu]

quick test locally that the trigger function I added works!!!

[peterdudfield]

Could you do something like using a DDL class. It's still raw sql but it the in the model code, not the migration. It would be nice not to rely on the migration code for database structure https://stackoverflow.com/questions/7888846/trigger-in-sqlachemyOn 19 Apr 2023 03:27, Eric Liu @.***> wrote: @ericcccsliu commented on this pull request.

In alembic/versions/8384de6b7c50_add_support_for_differing_inverterand.py:

+# revision identifiers, used by Alembic. +revision = '8384de6b7c50' +down_revision = 'aeea08bcfc42' +branch_labels = None +depends_on = None + + +def upgrade() -> None:

• op.add_column('sites', sa.Column('module_capacity_kw', sa.Float(), nullable=True, comment='The PV module nameplate capacity of the site'))
• op.add_column('sites', sa.Column('inverter_capacity_kw', sa.Float(), nullable=True, comment='The PV module nameplate capacity of the site'))
• op.execute("UPDATE sites SET inverter_capacity_kw=capacity_kw")

• Create a trigger function to set inverter_capacity_kw to capacity_kw

• op.execute("""
• CREATE OR REPLACE FUNCTION set_default_inverter_capacity_kw()

From what I can gather, it doesn't seem like alembic has support for adding triggers—everything that I can find recommends adding triggers using raw SQL. I might have missed something though—let me know if there's anything that I can do!

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: @.***>

[ericcccsliu]

Could you do something like using a DDL class. It's still raw sql but it the in the model code, not the migration. It would be nice not to rely on the migration code for database structure https://stackoverflow.com/questions/7888846/trigger-in-sqlachemyOn 19 Apr 2023 03:27, Eric Liu @.***

I tried refactoring my code, putting the following in sqlmodels:

set_default_inverter_capacity_kw_function = DDL("""
    CREATE OR REPLACE FUNCTION set_default_inverter_capacity_kw()
    RETURNS TRIGGER AS $$
    BEGIN
        NEW.inverter_capacity_kw := NEW.capacity_kw;
        RETURN NEW;
    END;
    $$ LANGUAGE plpgsql;
""")

event.listen(
    SiteSQL.__table__,
    "after_create",
    set_default_inverter_capacity_kw_function.execute_if(dialect="postgresql")
)

set_default_inverter_capacity_kw_trigger = DDL("""
    CREATE TRIGGER set_default_inverter_capacity_kw_trigger
    BEFORE INSERT ON sites
    FOR EACH ROW
    WHEN (NEW.inverter_capacity_kw IS NULL)
    EXECUTE FUNCTION set_default_inverter_capacity_kw();
""")

event.listen(
    SiteSQL.__table__,
    "after_create",
    set_default_inverter_capacity_kw_trigger.execute_if(dialect="postgresql")

But weirdly, this wouldn't work. Dug a little deeper and, unfortunately, it seems like Alembic only emits a table level after_create event—more info is in this stackoverflow thread. For our case, it seems like it might be easiest to leave the trigger in the migrations.

if these events are only for this one specific table though, then you wouldn't use any events, you'd just put the DDL() for those triggers directly in your migration script, right after where it calls create_table().