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

Add obs_raw indexes #89

Merged
merged 7 commits into from
May 11, 2021
Merged

Add obs_raw indexes #89

merged 7 commits into from
May 11, 2021

Conversation

rod-glover
Copy link
Contributor

@rod-glover rod-glover commented May 7, 2021

As a partial address to #87 , add the following indexes.

  • mod_time_idx
  • obs_raw_comp_idx
  • obs_raw_history_id_idx
  • obs_raw_id_idx

The new revision follows 7a3b247c577b (current head).

Index ORM Revision Test verify Done in prod
mod_time_idx x x x x
obs_raw_comp_idx x x x x
obs_raw_history_id_idx x x x x
obs_raw_id_idx x x x x

@rod-glover
Copy link
Contributor Author

Peculiar; the same tests (py.test -m "not slow" -v --tb=short tests) pass on my pytest-local Docker instance.

@rod-glover rod-glover requested a review from corviday May 11, 2021 17:00
@jameshiebert
Copy link
Contributor

Should be good to go here.

@jameshiebert jameshiebert merged commit 37e34f8 into master May 11, 2021
@jameshiebert jameshiebert deleted the i87-add-indexes branch May 11, 2021 17:20
Copy link
Contributor

@corviday corviday left a comment

Choose a reason for hiding this comment

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

Wow, the changes this PR makes are pretty straightforward, but there are just a gajillion contiguencies - different database starting states, different test setups, environments, etc. A lot of complexity, but broad, rather than deep, handling various contingencies and combinations.

It looks great to me, very thoughtful and thorough, though I am not familiar enough with this codebase to guess at what setups or states might additionally need to be covered, which seems the likeliest way there'd be an error here.

logger.debug(f"Creating index {index_name}")
op.create_index(index_name, table_name, columns, unique=False, schema=schema_name)
else:
logger.debug(f"Index {index_name} already exists; skipping create")
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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