Patch db update#854
Conversation
Update logger and settings to remove duplicated code and add in a pro…
Update logger and settings to remove duplicated code and add in a pro…
Update logger and settings to remove duplicated code and add in a pro…
Final tweaks to the critical error bug catching code to provide detai…
fix uncertain position parsing of extracted variants errors
Correct citation formatting and punctuation in README.
Merge pull request #847 from openvar/transcript_map_data_bug Fix a bug identified in batch processing
Added information about testing methodology and regression tests.
Added an 'About' section to provide context on VariantValidator.
Update format converter tests to use batch_list removed from vval()
In the uncorrected code, good data were not setting a variable to True. This meant that the fell back into a post populate and only if they could be corrected were they kept. This added slow processing and likely caused a lot of records to be omitted. Now corrected and tested in test_update_vv_db.py
Now passing batch_list and added additional tests
3 bugs identified. Tests added to cover the bugs and these add regression testing. Also, a functional test to cover refseq_check. The bugs prevented gene_symbol_catch from returning correctly, and missed a fallback position in refseq_catch.
Updates the vvDBInit.py code to better handle both MySQl and MariaDB. More stable and also used mapped error handling to enable testing regardless of the DB engine on the OS
We periodically see non-reproducible db fetches. The commit adds in a re-connect to the database for several attempts before raising to mitigate these. It also adds extra coverage to key modules
2 missing imports identified. Regression tests added
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 1081037 | Triggered | Generic Password | 45ab81f | db_dockerfiles/vdb/Dockerfile | View secret |
| 1081033 | Triggered | Generic Password | 5fd599d | configuration/docker-local.ini | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #854 +/- ##
===========================================
+ Coverage 77.11% 81.10% +3.98%
===========================================
Files 37 37
Lines 13071 13208 +137
===========================================
+ Hits 10080 10712 +632
+ Misses 2991 2496 -495 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Yes, this is because I fixed some bugs, during a huge uplift of coverage. I may have lost a few lines, but gained several hundred. Is this a fail? Nope!!!! |
Object creation for p. variants affecting the first aa were incorrect. Corrected and regression tests added. Also tests for interval start > interval end and additional tests for vvMixinCore.py. In addition, some unused imports have been removed from various modules
Uncertain positions without a range were causing a variant posedit creation error. Now removed. In some instances, fuzzy end posedit cannot be created so we keep a string. Now still raises exceptions. Fuzzy error exceptions now added to warnings. Additional bug fix allows validation if vvMixinCore.py `for gen in variant.primary_assembly_loci.keys()` where the variable is None. Tests and regression tests added to test_complex_descriptions.py
Bud identified in hgvs_utils.py. Incorrect kwarg into the function hgvs_delins_parts_to_hgvs_obj. Additional tests and regression tests added. Also additional coverage for complex_descriptions.py
Ensembl rest function times out or fails repeatedly for tests that pass. Add retries to ensure records can update
There has been a breaking change in the Ensembl TARK url. Update to new format
Retries added to all API calls to prevent missing data. Preferable over a timeout. Also, specific tests added to prevent not knowing if an API endpoint changes e.g. the recent change of TARK endpoint
Allele code passing into expanded_repeats.py was returning a non-parsable output, so could not be reached. This commit add code that ensures the expanded_repeats return is converted into the delins format and also, in this case adds the genomic reference so that intronic variants will pass To Do. Add code that can handle expanded repeats in alleles with the NC_ (NM_) format. But needs HGVS to confirm the new syntax. Also, investigate whether it is actually possible todo the conversion back from delins to expanded repeat from within an allele variant. Also add a warning that p. alleles are not supported or add in support.
John-F-Wagstaff
left a comment
John-F-Wagstaff
left a comment
There was a problem hiding this comment.
I am not nearly as experienced with the VV side of the DB code, but this looks good.
It is however getting quite large in patch number, we probably want to put this in and start a new one even if you have more bug fixes.
The only 2 things I might object to are firstly that a few of these patches would be better split up into smaller more focused patches, and secondly putting strings into the unknown spanned variants. Neither of these are worth holding this back for, so I think this is good to go in.
Using strings in hgvs objects will make the code more fragile and less clear so it its in some ways going backwards, which is not what we want to do in the long run, hence my dislike, but it works for now. A better fix is probably best left to a follow up. (We might want to put some of the tweaked HGVS object changes in the hgvs util code into vvhgvs given recent discussions and it would make sense to add accounting for unknown spans in at that point)
These bug fixes allow compound accessions to pass allele code. Also capture m. processing of alleles and currently set r. allele processing to unsupported. It also adds a configurable CONFIG_DIR which can be used by users, but mainly for testing of the config so that the system does not create corrupt files or cause race conditions when testing config manipulations. This also prevents sporadic overwriting of the ini file in when pytest fails
|
@John-F-Wagstaff. I have now added the last push to this. This stabilised the code and fixes 2 more small bugs. I agree, it bacame more of a task than expected and would have been better split up, but its all related to upping the coverage. As I upped, more and more bugs popped out. In terms of
This is a temp solution. I want to next, if time and patience allows, start adding helper functions to remove regulat Regex and correctly implement HGVS parsing throughout the codebase, so this patch (all be it huge) focusses on code stability and code coverage, the next I do will look at the regexes. I think merge this first, then check it all still works with your VRS code. Then I can re-branch from develop. This all sound OK? |
|
Yes that sounds fine. I will rebase the VRS code to avoid a nasty merge, then I can go through the patches one a a time and check that everything still works, new tests included, and apply fix-ups if needed. Once that is finished we can push the VRS code. |
|
OK, merging now. Some of the new tests you may want to update. Feel free. The key benefits of this patch are
|
2 participants
No description provided.