FlowKit icon indicating copy to clipboard operation
FlowKit copied to clipboard

Remove database connection during query init

Open greenape opened this issue 5 years ago • 10 comments

Closes #390 (which was already closed, but this closes it even more)

I have:

  • [x] Formatted any Python files with black
  • [x] Brought the branch up to date with master
  • [x] Added any relevant Github labels
  • [ ] Added tests for any new additions
  • [ ] Added or updated any relevant documentation
  • [ ] Added an Architectural Decision Record (ADR), if appropriate
  • [x] Added an MPLv2 License Header if appropriate
  • [x] Updated the Changelog

Description

Removes the use of a database connection during query init:

  • Added a new @pre_flight decorator which Query subclasses may use to indicate a method should be run to confirm the query is runnable
  • Added a new preflight() method to query, which calls all applicable pre-flight check methods for the query
  • Queries should no longer require communication with the database during __init__, any checks that require database access must now be implemented as a method of the class and use the @pre_flight decorator
  • When specifying tables in Flowmachine, the events. prefix is no longer required.
  • Table no longer automatically infers columns from the database, they must be specified.

For now, pre_flight is triggered during an actual cache write of a query, and also right after creating the query object to be stored. This preserves the existing behaviour of queries that can't actually run erroring at submission in the API.

I think in future we'd want to move away from that, because db checks are relatively expensive and slow down query creation when creating lots of queries via the API. Is possible however that switching out the existing sqlalchemy connection to Postgres for an asyncio one would alleviate that quite a bit. I guess we could also introduce two distinct connection pools as well - one for babysitting executing queries, and a separate one for pre-flight checks.


  • To see the specific tasks where the Asana app for GitHub is being used, see below:
    • https://app.asana.com/0/0/1202108706043964

:rocket: This description was created by Ellipsis for commit 6888c06f7186245f86e7babf6aeddfea19625744

Summary:

Refactors query initialization in Flowmachine to improve performance by removing automatic database checks during object creation, using a new @pre_flight decorator and preflight() method.

Key points:

  • Introduced @pre_flight decorator for methods requiring DB checks.
  • Added preflight() method in Query class to trigger these checks.
  • Removed automatic DB connection on Query initialization.
  • Updated tests to adapt to the new query initialization process.

Generated with :heart: by ellipsis.dev

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a @pre_flight decorator for Query subclasses to validate query runnability.
    • Added a preflight() method in the Query class for executing pre-flight checks.
    • Enhanced caching schema with the addition of a new zero_cache table.
  • Changed Functionality

    • The API now supports categorical metrics for joined spatial aggregates.
    • FlowMachine requires Python version 3.11 or higher.
    • Adjusted various class locations and removed redundant columns in the TotalActivePeriodsSubscriber class.
    • Updated handling of geometry tables and improved type safety in spatial unit classes.
    • Simplified table references in tests and various components by removing the "events." prefix.
    • Refined the initialization of Table instances across multiple test cases to include explicit column definitions.
  • Bug Fixes

    • Resolved issues with async task cancellation during server shutdown and improved error handling for various functions.
    • Fixed a 500 error when retrieving the API specification from FlowAPI and ensured correct execution of FlowAuth migrations.
    • Enhanced error handling in action handlers and tests for pre-flight failures.
  • Documentation

    • Updated the CHANGELOG.md to reflect notable changes in the FlowKit project.

These updates enhance functionality, improve error handling, and ensure better documentation for users.

greenape avatar Oct 13 '20 11:10 greenape

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Passing run #22649 ↗︎

0 4 0 0 Flakiness 0

Details:

Update test_query_object_construction.test_construct_query.approved.txt
Project: FlowAuth Commit: 6888c06f71
Status: Passed Duration: 00:46 💡
Started: May 23, 2024 8:50 AM Ended: May 23, 2024 8:50 AM

Review all test suite changes for PR #3128 ↗︎

cypress[bot] avatar Oct 15 '20 09:10 cypress[bot]

Codecov Report

Attention: 18 lines in your changes are missing coverage. Please review.

Comparison is base (68ed446) 92.97% compared to head (a1ba213) 93.82%.

:exclamation: Current head a1ba213 differs from pull request most recent head 95de5d2. Consider uploading reports for the commit 95de5d2 to get more accurate results

Files Patch % Lines
...wmachine/features/utilities/events_tables_union.py 80.00% 3 Missing and 2 partials :warning:
...machine/flowmachine/core/server/action_handlers.py 76.92% 2 Missing and 1 partial :warning:
flowmachine/flowmachine/core/spatial_unit.py 66.66% 2 Missing and 1 partial :warning:
flowmachine/flowmachine/core/table.py 90.62% 1 Missing and 2 partials :warning:
flowmachine/flowmachine/core/preflight.py 95.74% 1 Missing and 1 partial :warning:
flowmachine/flowmachine/core/random.py 83.33% 0 Missing and 1 partial :warning:
...owmachine/features/utilities/event_table_subset.py 94.44% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3128      +/-   ##
==========================================
+ Coverage   92.97%   93.82%   +0.85%     
==========================================
  Files         263      280      +17     
  Lines       10302    10786     +484     
  Branches      835     1296     +461     
==========================================
+ Hits         9578    10120     +542     
+ Misses        596      527      -69     
- Partials      128      139      +11     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 15 '20 09:10 codecov[bot]

@ellipsis-dev review this

greenape avatar May 23 '24 08:05 greenape

OK! Reviewing this PR...


Responding to this comment by @greenape. For more information about Ellipsis, check the documentation.

ellipsis-dev[bot] avatar May 23 '24 08:05 ellipsis-dev[bot]