-
-
Notifications
You must be signed in to change notification settings - Fork 447
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
bernd-wechner
wants to merge
5
commits into
Open-Web-Analytics:master
Choose a base branch
from
bernd-wechner:patch-3
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Commits on Jun 24, 2020
-
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.
Configuration menu - View commit details
-
Copy full SHA for 78fb367 - Browse repository at this point
Copy the full SHA 78fb367View commit details
Commits on Jun 25, 2020
-
owa_coreAPI::entityFactory('base.configuration') needs OWA_DB_TYPE and this is not available until the config file is loaded.
Configuration menu - View commit details
-
Copy full SHA for a685d70 - Browse repository at this point
Copy the full SHA a685d70View commit details -
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.
Configuration menu - View commit details
-
Copy full SHA for 83cbe46 - Browse repository at this point
Copy the full SHA 83cbe46View commit details
Commits on Jun 27, 2020
-
Metrics cannot be loaded if now database type is kown
So don't. Load metrics that is. _loadMetrics relies on the database definition.
Configuration menu - View commit details
-
Copy full SHA for fcb684e - Browse repository at this point
Copy the full SHA fcb684eView commit details -
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.
Configuration menu - View commit details
-
Copy full SHA for d0379b5 - Browse repository at this point
Copy the full SHA d0379b5View commit details
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.