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.

Sign up for GitHub

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

Jump to bottom

🔊🐛 More verbose loader logging + loader fixes #570

Merged
merged 3 commits into from
Feb 5, 2021
Merged

🔊🐛 More verbose loader logging + loader fixes #570

merged 3 commits into from
Feb 5, 2021

Conversation

znatty22
Copy link
Member

@znatty22 znatty22 commented Feb 3, 2021 •
edited
Loading

Tested locally

Closes #545
Closes #569

@znatty22 znatty22 added the feature New functionality label Feb 3, 2021
@znatty22 znatty22 self-assigned this Feb 3, 2021
@znatty22 znatty22 force-pushed the better-loader-logging branch from bd9aaf5 to 20c3196 Compare February 3, 2021 01:42
@znatty22 znatty22 changed the title 🔊 Better loader logging 🔊 🐛 More verbose loader logging + loader fixes Feb 3, 2021
@znatty22 znatty22 force-pushed the better-loader-logging branch 2 times, most recently from 91aa9a0 to fe02268 Compare February 3, 2021 20:36
@znatty22 znatty22 marked this pull request as ready for review February 3, 2021 20:41
@znatty22 znatty22 requested a review from a team as a code owner February 3, 2021 20:41
fiendish
fiendish suggested changes Feb 4, 2021
View reviewed changes
kf_lib_data_ingest/etl/load/load_v1.py Outdated Show resolved Hide resolved
kf_lib_data_ingest/etl/load/load_v1.py Outdated Show resolved Hide resolved
kf_lib_data_ingest/etl/load/load_v1.py Outdated Show resolved Hide resolved
kf_lib_data_ingest/target_api_plugins/kids_first_dataservice.py Outdated Show resolved Hide resolved
kf_lib_data_ingest/target_api_plugins/kids_first_dataservice.py Outdated Show resolved Hide resolved
@znatty22 znatty22 force-pushed the better-loader-logging branch 3 times, most recently from 7fcfdeb to d9ea9e6 Compare February 5, 2021 19:14
@znatty22 znatty22 requested a review from fiendish February 5, 2021 19:17
fiendish
fiendish reviewed Feb 5, 2021
View reviewed changes
kf_lib_data_ingest/etl/load/load_v1.py
@@ -87,11 +87,13 @@ def __init__(

target = urlparse(target_url).netloc or urlparse(target_url).path
self.uid_cache_filepath = os.path.join(
cache_dir or os.getcwd(),
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this?

Copy link
Member Author

@znatty22 znatty22 Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Wellll, bc when you instantiate loader with cache_dir=None, it throws an exception bc the base Stage class uses it for ingest_output_dir. I went back and forth on this. If no dir is passed in should the Loader use cwd as a default to pass to base class or should base class handle the no dir and set it to cwd?

Copy link
Member Author

@znatty22 znatty22 Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Oh also, its not removed. I just moved setting cache_dir up earlier in the constructor

Copy link
Contributor

@fiendish fiendish Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

but it looks like this doesn't use the cache dir anymore for the uid cache file? it seems like this always puts self.uid_cache_filepath in the current dir instead of in the cache dir

Copy link
Member Author

Choose a reason for hiding this comment

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

Ohhh yea sorry I see what you're saying now. I accidentally removed that little important line altogether.

kf_lib_data_ingest/target_api_plugins/kids_first_dataservice.py Outdated Show resolved Hide resolved
@znatty22 znatty22 force-pushed the better-loader-logging branch 2 times, most recently from b474985 to 6e06c5e Compare February 5, 2021 22:28
@znatty22
🐛 Raise exception on ambiguous target ID
5dfe635
@znatty22
🔊 Better load stage logging (e.g. skipped entities, duplicates)
e31a63c
@znatty22
🐛 ♻️ Fix handling of None vals, add external id for m2m
679c5d0 
Handle None values for GF.size, GF.hashes
Use more relevant exception with msg for req vals
Fill in external_id for many to many association tables
@znatty22 znatty22 force-pushed the better-loader-logging branch from 6e06c5e to 679c5d0 Compare February 5, 2021 22:39
@znatty22 znatty22 requested a review from fiendish February 5, 2021 22:39
fiendish
fiendish approved these changes Feb 5, 2021
View reviewed changes
Copy link
Contributor

@fiendish fiendish left a comment

Choose a reason for hiding this comment

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

👑

@fiendish fiendish merged commit ee06380 into master Feb 5, 2021
@fiendish fiendish deleted the better-loader-logging branch February 5, 2021 23:48
@fiendish fiendish changed the title 🔊 🐛 More verbose loader logging + loader fixes 🔊🐛 More verbose loader logging + loader fixes Feb 5, 2021
fiendish added a commit that referenced this pull request Feb 6, 2021
@fiendish
🏷️ Release 1.0.0
744bfd9 
## Release 1.0.0

### New features and changes

- [#570](#570) -  🔊🐛 More verbose loader logging + loader fixes - [ee06380](ee06380) by [znatty22](https://github.com/znatty22)
- [#564](#564) -  🐛 Validate V1 API configurations - [41dfe3c](41dfe3c) by [fiendish](https://github.com/fiendish)
- [#568](#568) -  ♻️ Don't silently cross-assign SE.SL/SE.ID - [002b252](002b252) by [fiendish](https://github.com/fiendish)
- [#523](#523) -  📝 Update documentation for recent changes - [f84e272](f84e272) by [fiendish](https://github.com/fiendish)
- [#567](#567) -  🐛 Don't nullify coerced integers - [5439e4e](5439e4e) by [fiendish](https://github.com/fiendish)
- [#566](#566) -  💬 Add FredHutch sequencing center - [fe49db9](fe49db9) by [fiendish](https://github.com/fiendish)
- [#563](#563) -  🔥 Remove test files from template project - [0653622](0653622) by [fiendish](https://github.com/fiendish)
- [#562](#562) -  🔧 Add marrow procurement to validator - [c7ccbd5](c7ccbd5) by [fiendish](https://github.com/fiendish)
- [#561](#561) -  🔥 Remove dead css file from doc generation - [f1b24c6](f1b24c6) by [fiendish](https://github.com/fiendish)
- [#555](#555) -  ➖ Use a mutex instead of sqlite3worker - [d73961c](d73961c) by [fiendish](https://github.com/fiendish)
- [#554](#554) -  🐛 Ensure integers fit into allocated bits - [b6ad332](b6ad332) by [fiendish](https://github.com/fiendish)
- [#553](#553) -  💬 Add maf format to constants - [31266e8](31266e8) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#552](#552) -  ✨ Add option to query from another source in Load - [04f0310](04f0310) by [fiendish](https://github.com/fiendish)
- [#551](#551) -  💬 Add sickkids sequencing center - [e250fbb](e250fbb) by [youngnm](https://github.com/youngnm)
- [#550](#550) -  💬 New constant for marrow aspiration - [f82589b](f82589b) by [fiendish](https://github.com/fiendish)
- [#548](#548) -  🐛 Remove non-member fields before submitting - [0310c62](0310c62) by [fiendish](https://github.com/fiendish)
- [#547](#547) -  ✏️ Fix BC_CANCER_AGENCY typo - [dd3f227](dd3f227) by [fiendish](https://github.com/fiendish)
- [#549](#549) -  ✨ Convert family relations to P1+P2+rel - [f5ce3d2](f5ce3d2) by [fiendish](https://github.com/fiendish)
- [#546](#546) -  💬 Add string constant for html type - [d201dfa](d201dfa) by [fiendish](https://github.com/fiendish)
- [#541](#541) -  🐛 Reference participant/study in more service plugin key components - [fe5546f](fe5546f) by [fiendish](https://github.com/fiendish)
- [#542](#542) -  💚 Fix sphinx theme install egg name - [0935579](0935579) by [fiendish](https://github.com/fiendish)
- [#539](#539) -  📌 Make some of the dependencies flexible - [f519be3](f519be3) by [fiendish](https://github.com/fiendish)
- [#537](#537) -  🔊 Add html validation report - [2fe259e](2fe259e) by [fiendish](https://github.com/fiendish)
- [#536](#536) -  🐛 Escape strings before adding to markdown - [80be37f](80be37f) by [fiendish](https://github.com/fiendish)
- [#532](#532) -  ⬆️ Update dependencies for Python 3.9 - [3802a55](3802a55) by [fiendish](https://github.com/fiendish)
- [#533](#533) -  ✨ Add Ashion Sequencing center to constants.py - [a0d11b6](a0d11b6) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#531](#531) -  🐛 Fake-cache target IDs in dry run mode - [5352c24](5352c24) by [fiendish](https://github.com/fiendish)
- [#529](#529) -  🐛 Fix bad call order blocking BS and SE loads - [2d14c3e](2d14c3e) by [fiendish](https://github.com/fiendish)
- [#527](#527) -  🐛 Don't show blank cells as invalid values - [7eb86a4](7eb86a4) by [fiendish](https://github.com/fiendish)
- [#522](#522) -  🐛 Don't contact server during dry run testing - [c802ab5](c802ab5) by [fiendish](https://github.com/fiendish)
- [#524](#524) -  👽 Add auth configs for new study creator subdomains - [027b60c](027b60c) by [fiendish](https://github.com/fiendish)
- [#514](#514) -  🐛 Correct study ID/TARGET_SERVICE_ID overwrite - [4ba24ae](4ba24ae) by [fiendish](https://github.com/fiendish)
- [#513](#513) -  ♻️ Break dependency on local ID cache - [b70f26f](b70f26f) by [fiendish](https://github.com/fiendish)
- [#521](#521) -  ✨ Add hmb-npu to constants.CONSENT_TYPE - [4151df8](4151df8) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#520](#520) -  🎨 Display test result summary as table - [d8346e8](d8346e8) by [znatty22](https://github.com/znatty22)
- [#518](#518) -  📝 Consolidate example extract config copies - [a453461](a453461) by [fiendish](https://github.com/fiendish)
- [#517](#517) -  📝 Import Split() in docs extract config - [a3ecfdd](a3ecfdd) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#495](#495) -  💬 Add concepts for participant template - [7c319f4](7c319f4) by [fiendish](https://github.com/fiendish)
- [#499](#499) -  ✨ Data Validation - [20ac772](20ac772) by [znatty22](https://github.com/znatty22)
- [#509](#509) -  🐛 Detect encodings of files when reading - [263d1bf](263d1bf) by [fiendish](https://github.com/fiendish)
- [#497](#497) -  🐛 Family ID is optional for Participants - [7944c6e](7944c6e) by [fiendish](https://github.com/fiendish)
- [#487](#487) -  🐛 Remove bool json type cast - [794a387](794a387) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#492](#492) -  🔥 Remove old accounting code - [1ebcb1f](1ebcb1f) by [znatty22](https://github.com/znatty22)
- [#482](#482) -  👷 Migrate to gh actions - [5b6d4a5](5b6d4a5) by [blackdenc](https://github.com/blackdenc)
- [#484](#484) -  ✨ Add sequencing centers - [6cc18b5](6cc18b5) by [liberaliscomputing](https://github.com/liberaliscomputing)
- [#485](#485) -  💚 Unbreak tests on Circle - [f7ef977](f7ef977) by [fiendish](https://github.com/fiendish)
- [#472](#472) -  ⬆️ Upgrade click dependency - [69175af](69175af) by [fiendish](https://github.com/fiendish)
- [#470](#470) -  📝 Add example target plugins to the tutorial - [7fd17eb](7fd17eb) by [fiendish](https://github.com/fiendish)
- [#469](#469) -  👷 Automate tagging releases - [d05b4da](d05b4da) by [fiendish](https://github.com/fiendish)
@fiendish fiendish mentioned this pull request Feb 6, 2021
Closed
fiendish added a commit that referenced this pull request Feb 6, 2021
@fiendish
🏷️ Release 1.0.0
97242b1 
## Release 1.0.0

### Summary

- Emojis: 🐛 x15, 🔊 x2, ♻️ x2, 📝 x4, 💬 x6, 🔥 x3, 🔧 x1, ➖ x1, ✨ x6, ✏️ x1, 💚 x2, 📌 x1, ⬆️ x2, 👽 x1, 🎨 x1, 👷 x2
- Categories: Additions x8, Documentation x4, Removals x4, Fixes x23, Ops x7, Other Changes x4

### New features and changes

- [#570](#570) - 🔊🐛 More verbose loader logging + loader fixes - [ee06380](ee06380) by [znatty22](https://github.com/znatty22)
- [#564](#564) - 🐛 Validate V1 API configurations - [41dfe3c](41dfe3c) by [fiendish](https://github.com/fiendish)
- [#568](#568) - ♻️ Don't silently cross-assign SE.SL/SE.ID - [002b252](002b252) by [fiendish](https://github.com/fiendish)
- [#523](#523) - 📝 Update documentation for recent changes - [f84e272](f84e272) by [fiendish](https://github.com/fiendish)
- [#567](#567) - 🐛 Don't nullify coerced integers - [5439e4e](5439e4e) by [fiendish](https://github.com/fiendish)
- [#566](#566) - 💬 Add FredHutch sequencing center - [fe49db9](fe49db9) by [fiendish](https://github.com/fiendish)
- [#563](#563) - 🔥 Remove test files from template project - [0653622](0653622) by [fiendish](https://github.com/fiendish)
- [#562](#562) - 🔧 Add marrow procurement to validator - [c7ccbd5](c7ccbd5) by [fiendish](https://github.com/fiendish)
- [#561](#561) - 🔥 Remove dead css file from doc generation - [f1b24c6](f1b24c6) by [fiendish](https://github.com/fiendish)
- [#555](#555) - ➖ Use a mutex instead of sqlite3worker - [d73961c](d73961c) by [fiendish](https://github.com/fiendish)
- [#554](#554) - 🐛 Ensure integers fit into allocated bits - [b6ad332](b6ad332) by [fiendish](https://github.com/fiendish)
- [#553](#553) - 💬 Add maf format to constants - [31266e8](31266e8) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#552](#552) - ✨ Add option to query from another source in Load - [04f0310](04f0310) by [fiendish](https://github.com/fiendish)
- [#551](#551) - 💬 Add sickkids sequencing center - [e250fbb](e250fbb) by [youngnm](https://github.com/youngnm)
- [#550](#550) - 💬 New constant for marrow aspiration - [f82589b](f82589b) by [fiendish](https://github.com/fiendish)
- [#548](#548) - 🐛 Remove non-member fields before submitting - [0310c62](0310c62) by [fiendish](https://github.com/fiendish)
- [#547](#547) - ✏️ Fix BC_CANCER_AGENCY typo - [dd3f227](dd3f227) by [fiendish](https://github.com/fiendish)
- [#549](#549) - ✨ Convert family relations to P1+P2+rel - [f5ce3d2](f5ce3d2) by [fiendish](https://github.com/fiendish)
- [#546](#546) - 💬 Add string constant for html type - [d201dfa](d201dfa) by [fiendish](https://github.com/fiendish)
- [#541](#541) - 🐛 Reference participant/study in more service plugin key components - [fe5546f](fe5546f) by [fiendish](https://github.com/fiendish)
- [#542](#542) - 💚 Fix sphinx theme install egg name - [0935579](0935579) by [fiendish](https://github.com/fiendish)
- [#539](#539) - 📌 Make some of the dependencies flexible - [f519be3](f519be3) by [fiendish](https://github.com/fiendish)
- [#537](#537) - 🔊 Add html validation report - [2fe259e](2fe259e) by [fiendish](https://github.com/fiendish)
- [#536](#536) - 🐛 Escape strings before adding to markdown - [80be37f](80be37f) by [fiendish](https://github.com/fiendish)
- [#532](#532) - ⬆️ Update dependencies for Python 3.9 - [3802a55](3802a55) by [fiendish](https://github.com/fiendish)
- [#533](#533) - ✨ Add Ashion Sequencing center to constants.py - [a0d11b6](a0d11b6) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#531](#531) - 🐛 Fake-cache target IDs in dry run mode - [5352c24](5352c24) by [fiendish](https://github.com/fiendish)
- [#529](#529) - 🐛 Fix bad call order blocking BS and SE loads - [2d14c3e](2d14c3e) by [fiendish](https://github.com/fiendish)
- [#527](#527) - 🐛 Don't show blank cells as invalid values - [7eb86a4](7eb86a4) by [fiendish](https://github.com/fiendish)
- [#522](#522) - 🐛 Don't contact server during dry run testing - [c802ab5](c802ab5) by [fiendish](https://github.com/fiendish)
- [#524](#524) - 👽 Add auth configs for new study creator subdomains - [027b60c](027b60c) by [fiendish](https://github.com/fiendish)
- [#514](#514) - 🐛 Correct study ID/TARGET_SERVICE_ID overwrite - [4ba24ae](4ba24ae) by [fiendish](https://github.com/fiendish)
- [#513](#513) - ♻️ Break dependency on local ID cache - [b70f26f](b70f26f) by [fiendish](https://github.com/fiendish)
- [#521](#521) - ✨ Add hmb-npu to constants.CONSENT_TYPE - [4151df8](4151df8) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#520](#520) - 🎨 Display test result summary as table - [d8346e8](d8346e8) by [znatty22](https://github.com/znatty22)
- [#518](#518) - 📝 Consolidate example extract config copies - [a453461](a453461) by [fiendish](https://github.com/fiendish)
- [#517](#517) - 📝 Import Split() in docs extract config - [a3ecfdd](a3ecfdd) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#495](#495) - 💬 Add concepts for participant template - [7c319f4](7c319f4) by [fiendish](https://github.com/fiendish)
- [#499](#499) - ✨ Data Validation - [20ac772](20ac772) by [znatty22](https://github.com/znatty22)
- [#509](#509) - 🐛 Detect encodings of files when reading - [263d1bf](263d1bf) by [fiendish](https://github.com/fiendish)
- [#497](#497) - 🐛 Family ID is optional for Participants - [7944c6e](7944c6e) by [fiendish](https://github.com/fiendish)
- [#487](#487) - 🐛 Remove bool json type cast - [794a387](794a387) by [chris-s-friedman](https://github.com/chris-s-friedman)
- [#492](#492) - 🔥 Remove old accounting code - [1ebcb1f](1ebcb1f) by [znatty22](https://github.com/znatty22)
- [#482](#482) - 👷 Migrate to gh actions - [5b6d4a5](5b6d4a5) by [blackdenc](https://github.com/blackdenc)
- [#484](#484) - ✨ Add sequencing centers - [6cc18b5](6cc18b5) by [liberaliscomputing](https://github.com/liberaliscomputing)
- [#485](#485) - 💚 Unbreak tests on Circle - [f7ef977](f7ef977) by [fiendish](https://github.com/fiendish)
- [#472](#472) - ⬆️ Upgrade click dependency - [69175af](69175af) by [fiendish](https://github.com/fiendish)
- [#470](#470) - 📝 Add example target plugins to the tutorial - [7fd17eb](7fd17eb) by [fiendish](https://github.com/fiendish)
- [#469](#469) - 👷 Automate tagging releases - [d05b4da](d05b4da) by [fiendish](https://github.com/fiendish)
@fiendish fiendish mentioned this pull request Feb 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fiendish fiendish fiendish approved these changes

Assignees

@znatty22 znatty22

Labels
feature New functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

More graceful handling of GF missing required attributes Need more insight into skipped loads
2 participants
@znatty22 @fiendish