Conversation
…uous_Pressure_Daily Context: updates db/nma_legacy.py to map GlobalID and WellID as UUID(as_uuid=True) and documents the WellID FK to Thing for the continuous pressure daily model.
- Define read-only Starlette Admin view with full legacy-order fields - Register view in admin/views/__init__.py and admin/config.py
…acy.py for UUID IDs - Updated the test helper to return UUIDs for GlobalID and WellID in legacy model tests - Changed well_id to use a UUID so it matches the model mapping.
…int in nma_legacy model
…constraint in nma_legacy model
…ntinuous pressure daily records
- Added migrations for thing_id FK and UUID column alignment - Updated pressure daily legacy tests for Thing linkage and FK enforcement
- Cache Thing IDs and map PointID to thing_id to satisfy new FK - Filter orphan rows to prevent invalid inserts - Add focused transfer unit test to validate mapping and filtering
…elsContinuous_Pressure_Daily
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements cleanup and refactoring changes to the data transfer pipeline, focusing on improving code organization, adding limit functionality for testing, and fixing data type handling issues.
Changes:
- Added LIMIT flag support to reduce dataset sizes during parallel transfers for testing purposes
- Fixed data type conversion in stratigraphy legacy transfers (StratTop/StratBottom now use int instead of float)
- Added WCLab_ID field to minor trace chemistry transfer with test coverage
- Reorganized transfer options loading and improved logging configuration in alembic
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| transfers/well_transfer.py | Added LIMIT flag support to restrict dataframe size in parallel transfers |
| transfers/transfer.py | Implemented limit scaling logic for parallel transfers and reorganized transfer options initialization |
| transfers/stratigraphy_legacy.py | Changed StratTop/StratBottom fields from float to int conversion |
| transfers/minor_trace_chemistry_transfer.py | Added WCLab_ID field to row dict and upsert statement |
| transfers/logger.py | Separated log filename from path and removed commented code |
| tests/test_minor_trace_chemistry_transfer.py | Added test coverage for WCLab_ID field inclusion |
| alembic/env.py | Improved logging configuration with environment variable control and better handler inheritance |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1f00e1b6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…uousDaily' into BDMS-520-1-1-Cleanup-2.0
…refactor transfer logic for parallel execution
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
tests/test_waterlevelscontinuous_pressure_daily_legacy.py:2
- The copyright year is set to 2026, which is in the future. This should be updated to the current year (2025 or earlier).
# Copyright 2026 ross
# Conflicts: # db/nma_legacy.py # db/thing.py # transfers/minor_trace_chemistry_transfer.py # transfers/stratigraphy_legacy.py # transfers/transfer.py
…ace_chemistry_transfer
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?