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

Fueling discussion: Is it sensible to assume mysql during setup? #634

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Commits on Jun 24, 2020

  1. Fueling discussion: Is it sensible to assume myseql during setup?

    This came to my attention because if I select a postgresql database (am experimenting with that) the installer ends up loading the mysql configuration  then the postgresql configuration. That throws a pile of non fatal errors on my console (hence is noticeable). But raises some questions.
    
    The first load (before the db_type is known) is with an explicit entity creation for base.configuration at line 60 in settings.php, and the second load (now knowing the db_type) on line 80 via a call to setupStorageEngine.
    
    There is inconsistent methodology for ensuring a single load as well, with entityFactory consulting OWA_DTD_INT (an arbitrary variable known to be set in the database plugin file) to determine if the file's been loaded and setupStorageEngine using a require_once strategy. A no-no, consistency of approach is desirable.
    
    Questions on the table:
    
    1. Does entityFactory need to setup the Storage engine at all?
    2. Does it need to setup a Storage engine for all entities or only some?
    3. Is it ever kosher to fall back on mysql (IMO no)? Or is it better to bail? Or can we, must we find a way to order things so that fallback is never needed?
    
    This is NOT a complete PR. There is a BIG issue at play still in that:
    
    1. the constructor for owa_configuration requires database information.
    2. this information is not yet available for the selected database when it is called in the owa_settings constructor
    3. currently this falls back to mysql (the bit this PR changes), but if we don't fall back to something, the configuration constructor fails.
    4. I've tried delaying this database configuration in the configuration object but things go wrong fast. It will take more work to understand the nuance of ordering here. 
    5. what is clear is the a configurable database engine revels a strange race condition here, that needs resolving methinks (in which the configuration object needs to know about the database but it it doesn't know what type of database yet. 
    
    Am hoping more experience eyes can resolve this and comment. For now this is a very short PR to fuel that thinking. If an issue is more appropriate just say, no problem there. Just figured could work on and evolve this PR to resolve the issue.
    bernd-wechner committed Jun 24, 2020
    Configuration menu
    Copy the full SHA
    78fb367 View commit details
    Browse the repository at this point in the history

Commits on Jun 25, 2020

  1. Fixing an ordering bug

    owa_coreAPI::entityFactory('base.configuration') needs OWA_DB_TYPE and this is not available until the config file is loaded.
    bernd-wechner committed Jun 25, 2020
    Configuration menu
    Copy the full SHA
    a685d70 View commit details
    Browse the repository at this point in the history
  2. Can safely assume OWA_DB_TYPE is set

    Thanks to the fix in settings.py this can now safely assume that OWA_DB_TYPE is set.
    bernd-wechner committed Jun 25, 2020
    Configuration menu
    Copy the full SHA
    83cbe46 View commit details
    Browse the repository at this point in the history

Commits on Jun 27, 2020

  1. Metrics cannot be loaded if now database type is kown

    So don't. Load metrics that is. _loadMetrics relies on the database definition.
    bernd-wechner committed Jun 27, 2020
    Configuration menu
    Copy the full SHA
    fcb684e View commit details
    Browse the repository at this point in the history
  2. Make robust against unavailable database definition

    User entity is needed prior to database definition being available when rendering the Configuration Settings page. It probably shouldn't be, but the easiest way to avoid errors for now, is to make a User entity available without data type definitions.  That satisfies the Configuration Settings page.
    bernd-wechner committed Jun 27, 2020
    Configuration menu
    Copy the full SHA
    d0379b5 View commit details
    Browse the repository at this point in the history