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

HDX-10241 Update VAT definitions to match Alembic migration scripts #71

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

IanHopkinson
Copy link
Contributor

In preparing Alembic migration scripts for the provider name change we noticed some inconsistencies in the primary key/index settings. This PR resolves those consistencies, or at least those that were apparent

One philosophical change is that we explicitly index the provider_admin1_name and provider_admin2_name columns. They typically belong to the primary key and are thus indexed, however the primary keys are composite and so search performance is not improved without explicitly indexing the column.

Copy link

Test Results

115 tests  ±0   115 ✅ ±0   53s ⏱️ +24s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit f8dceba. ± Comparison against base commit 339377c.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 11435605920

Details

  • 1 of 211 (0.47%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 83.51%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/hapi_schema/utils/view_as_table_code_generator.py 0 104 0.0%
src/hapi_schema/utils/hapi_views_code_generator.py 0 106 0.0%
Totals Coverage Status
Change from base Build 11355724549: 0.06%
Covered Lines: 1104
Relevant Lines: 1322

💛 - Coveralls

@davidmegginson
Copy link
Contributor

Thank you -- this sounds like a valuable change.

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.

3 participants