Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactors code to produce fim service inundation sublayers that now include the "model_version" field #949

Merged
merged 7 commits into from
Nov 8, 2024

Conversation

shawncrawley
Copy link
Collaborator

@shawncrawley shawncrawley commented Nov 7, 2024

Refs #824

Additions

  • No new files

Removals

  • Core/LAMBDA/viz_functions/viz_publish_service/services/catfim folder (this was an R&D effort that has been abandonded)
  • Core/LAMBDA/viz_functions/viz_stage_based_catfim folder (this was an R&D effort that has been abandonded)

Changes

  • Core/EC2/RDSBastion/main.tf
    • Removes unused fim_version variable
  • Core/main.tf
    • Adds hand_version (read from Terraform environment file) as variable to viz-functions module (i.e. Core/LAMBDA/viz_functions/main.tf)
  • Core/LAMBDA/layers/viz_lambda_shared_funcs/python/viz_classes.py
    • Renames/refactors run_sql_file_in_db to execute_sql for required use in
      Core/LAMBDA/viz_functions/viz_fim_data_prep/lambda_function.py (see below)
    • Renames run_sql_in_db to sql_to_dataframe because I've always wanted to for clarity's sake, so why not now when I had to refactor the other related function above
  • Core/LAMBDA/layers/viz_lambda_shared_funcs/python/viz_lambda_shared_funcs.py
    • Same two items as Core/LAMBDA/layers/viz_lambda_shared_funcs/python/viz_classes.py above (Not sure why this code is duplicated here in the first place...)
  • Core/LAMBDA/rnr_functions/rnr_domain_generator/lambda_function.py
    • Implements refactor of run_sql_in_db to sql_to_dataframe
  • Core/LAMBDA/viz_functions/main.tf
    • Now accepts and appropriately disseminates hand_version variable to viz_db_postprocess Lambda (i.e. Core/LAMBDA/viz_functions/viz_db_postprocess_sql/lambda_function.py) and image_based LAMBDAS module (i.e. Core/LAMBDA/viz_functions/image_based/main.tf).
  • Core/LAMBDA/viz_functions/image_based/main.tf
    • Now accepts hand_version in addition to originally accepting fim_version and dissemenates it to viz_hand_fim_processing Lambda (i.e. Core/LAMBDA/viz_functions/image_based/viz_hand_fim_processing/lambda_function.py) as HAND_PREFIX (renamed from FIM_PREFIX to be more clear) and FIM_VERSION environment variables.
    • The FIM_BUCKET envrionment variable of the viz_hand_fim_processing Lambda was also renamed to HAND_BUCKET for clarity.
  • Core/LAMBDA/viz_functions/image_based/viz_hand_fim_processing/lambda_function.py
    • Renames global FIM_* variables to HAND_* for clarity.
    • Writes hand_version in addition to fim_version to final pandas dataframe that gets uploaded to the product's {db_fim_table}.
  • Core/LAMBDA/viz_functions/viz_db_postprocess_sql/lambda_function.py
    • Now extracts FIM_VERSION from environment (as passed along from Core/LAMBDA/viz_functions/main.tf) and injects it into fim_caching_template SQL files.
  • Multiple files in Core/LAMBDA/viz_functions/viz_db_postprocess_sql/viz_db_postprocess_sql/fim_caching_templates:
    • 0_create_or_truncate_tables.sql
      • Adds model_version to column schema of {db_fim_table}
      • Adjusts the data types of fim_version and reference_time columns to varchar
    • 2a_query_fim_cache-ras2fim.sql
      • fim_version is now passed in from template variable, rather than pulling from table
    • 2b_query_fim_cache-hand.sql
      • fim_version is now passed in from template variable, rather than pulling from table
      • Updated to reflect that fim_cache schema will be renamed to handfim_cache
      • Minor reformatting
    • 3_add_any_processed_hand_fim_back_to_cache.sql
      • GROUP BY query modified to use DISTINCT for clarity and optimization
      • Reflects that fim_cache schema will be renamed to handfim_cache
      • Relflects that fim_version will be renamed to model_version in handfim_cache.hydrotable_cached_max table
      • Minor reformatting
    • 4_create_fim_config_publish_table.sql
      • model_version column added to query that produces {db_publish_table}
    • 5_dummy_rows_for_wpod_service_monitor.sql
      • model_version is now included in dummy row population
  • Core/LAMBDA/viz_functions/viz_db_postprocess_sql/fim_configs/rfc_based_5day_max_inundation.sql
    • model_version added to query that produced publish.rfc_based_5day_max_inundation table
  • Core/LAMBDA/viz_functions/viz_fim_data_prep/lambda_function.py
    • Removes unused code
    • Now pulls in FIM_VERSION environment variable
    • Implements refactor of run_sql_file_in_db to execute_sql (refactored to allow for template variable injection)
    • Injects {fim_version} into all rf_*_inundation AEP FIM scripts
  • Core/LAMBDA/viz_functions/viz_fim_data_prep/reference_preprocessing_sql/rf_*_inundation.sql
    • Refactored to inject fim_version from as template variable (passed down from Terraform)
  • Core/LAMBDA/viz_functions/viz_publish_service/services/*/*.mapx
    • Fully incorporates model_version into relevant service sublayers, from including it in the "sqlQuery" and adding entries for it in both the featureTable -> fieldDescriptions and queryFields lists.
    • Unifies the length property in the queryFields list entries for both the fim_version and model_version fields across all .mapx files. Uses 12 for fim_version and 20 for model_version

External Dependencies

The following dependencies are maintained by us, the HydroVIS team, but are maintained external to this code base. Thus, these constitute additional manual work that will need to be performed before the code in this PR will run appropriately.

  • The fim_version and hand_version must be provided as Terraform deployment environment variables (as stored in the owp-cloud-aws-hydrovis repository). The hand_version variable will now store what fim_version previously stored (since "fim_version" previously was an alias for "hand_version"). The fim_version now represent a composite or package FIM version, one number to account for both the HAND version and Ras2FIM version.
  • The fim_cache schema needs to be renamed to handfim_cache. This change is to clearly distinguish it from ras2fim (since both are fim)
  • The fim_version column of the hydrotable_cached_maxtable needs to be renamed tomodel_version`

Copy link
Contributor

@RobHanna-NOAA RobHanna-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not do a big comprehensive review, but Shawn and I talked through it. Not really something we can easily stub test. But.. looks good and is ready to deploy.

@shawncrawley shawncrawley merged commit 4842abc into ti Nov 8, 2024
1 check passed
@nickchadwick-noaa nickchadwick-noaa added this to the V2.1.8 milestone Jan 8, 2025
@nickchadwick-noaa nickchadwick-noaa linked an issue Jan 8, 2025 that may be closed by this pull request
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new "model_version" field to all inundation services/sublayers
3 participants