-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/tapdb integration #159
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Addresses second round of ChatGPT 5.2 feedback:
1. Transaction contradiction resolved:
- get_session() → library never commits (caller responsible)
- session_scope(commit=True/False) → opt-in convenience
2. Audit log verbosity documented:
- One row per changed column per UPDATE
- Soft DELETE = 1 DELETE + 2 UPDATE rows
- Partitioning recommendation for large deployments
3. Loader semantics specified:
- Upsert behavior (skip unchanged, error on changed)
- Version bump rule for modifications
- instance_prefix resolution order
- Conflict behavior documented
4. Action group naming standardized:
- Canonical pattern: {btype}_actions
- materialize_actions() updated to use pattern
5. Auto-created action templates clarified:
- Audit-only stubs (no action_definition)
- Suitable for: audit linkage, EUID prefix, cate
Addresses second round of ChatGPT 5.2 feedback:
1. Transaction contradiction resolved:
- get_session() → library never commits (caller responsible)
1. Transaction contradiction resolved:
- gns
- get_session() → library never c - session:
cd /Users/daylily/projects/daylily_repos/bloom && git status --short
- Added -e ../daylily-tapdb to bloom_env.yaml pip dependencies - Enables BLOOM to import daylily_tapdb module - Validation: python -c 'import daylily_tapdb' succeeds
- Created bloom_lims/tapdb_adapter.py with: - SQLAlchemy synonyms: super_type↔category, btype↔type, b_sub_type↔subtype - Class aliases: file_set_instance→file_instance, etc. - BLOOMdb3 wrapper preserving BLOOM's public API - All TapDB models re-exported with BLOOM-compatible names - Validation: imports compile successfully
- Renamed bloom_lims/db.py → bloom_lims/db_legacy.py - Created new bloom_lims/db.py as thin façade re-exporting from tapdb_adapter - All existing imports (from bloom_lims.db import BLOOMdb3) continue to work - Preserved utility functions (generate_random_string, get_datetime_string, etc.) - Validation: python -m compileall bloom_lims succeeds
- Created bloom_lims/env/bloom_prefix_sequences.sql with all 21 BLOOM prefixes (CX, CWX, MX, MRX, MCX, EX, WX, AY, WSX, QX, DX, TRX, AX, XX, FG, FI, FS, FX, SX, EV, GX) - Updated bloom_lims/env/install_postgres.sh to: - Apply TapDB schema from ../daylily-tapdb/schema/tapdb_schema.sql - Apply BLOOM prefix sequences for EUID generation - TAPDB_SCHEMA_SQL env var allows overriding schema path - Validation: all 21 sequence definitions verified
- Removed grep/CASE check in seed_db_containersGeneric.py (TapDB uses sequence-based EUID generation, no CASE expressions in schema) - Updated README.md reference from postgres_schema_v3.sql to tapdb_schema.sql - Validation: seed script compiles successfully
- Deleted bloom_lims/db_legacy.py (original BLOOM db implementation) - Deleted bloom_lims/env/postgres_schema_v3.sql (legacy schema) - Updated migration docs to reference TapDB schema instead of legacy schema: - bloom_lims/migrations/__init__.py - bloom_lims/migrations/utils.py - bloom_lims/migrations/versions/20241223_0001_initial_baseline.py - Validation: grep -R 'postgres_schema_v3.sql' --include='*.py' returns zero matches - Validation: python -m compileall bloom_lims succeeds
SQLAlchemy synonyms only work for attribute access on existing objects, not for constructor keyword arguments. Added _BloomCompatibleClassWrapper that intercepts class instantiation to translate BLOOM field names (super_type, btype, b_sub_type) to TapDB field names (category, type, subtype). All classes registered via _register_orm_classes() are now wrapped with this compatibility layer, enabling seed scripts and other code to use BLOOM field names when creating new records.
SQLAlchemy synonym() only works for instance attribute access, not for filter expressions in queries. Replaced with hybrid_property which properly generates SQL column expressions for both: - Instance access: obj.super_type reads/writes category - Query filters: Model.super_type == 'file' generates WHERE category = 'file' This fixes the IndexError in file creation tests where query_template_by_component_v2() was returning empty results because the filter expressions were evaluating to Python False instead of SQL expressions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.