refactor!: Database #16961
Merged
refactor!: Database #16961
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gavindsouza
force-pushed
the
mariadb-client-refactor
branch
3 times, most recently
from
6a0238e
to
0baac56
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Move to use MariaDB's official Python client written in C instead of the PyMySQL library. This change doesn't rid Frappe of the PyMySQL library. Instead, it continues to utilize it for the ER module and converter methods until the MariaDB library adds support for the same. Ticket: https://jira.mariadb.org/projects/CONPY/issues/CONPY-203
* Make all methods static * Add typing hints * Don't safe fetch attribute value for errno - MariaDB exceptions if raised will have errno attr in them. If the class doesn't have one, it's not meant to be passed in these methods.
This change has only been done to separate and club only like methods / utils together
* Drop mandatory unused arg in method call * Use pluck instead of list comprehension + subscripting
Start initial pool of 4 (_POOL_SIZE) connections for a given site. When the pool is exhausted, generate new connections upon request and add them to the pool. Max allowed size for each pool is 64 (_MAX_POOL_SIZE) connections. However, you may have more than 64 active connections, just that they won't be pooled but destroyed upon job completion/end of request/similar cycle.
For the times you don't want to use pooling ;) Calling get_connection/connect will close all pooled connections for the current process / worker!
_SITE_POOLS[frappe.local.site].read_only will hold ConnectionPool for replica database connections. _SITE_POOLS[frappe.local.site].default will hold pool for connections that allow read+writes
Help table has been deprecated for a while. It's safe to get rid of the API too.
This logic mirror how replica connections are handled
…database_connection_pooling Also, don't pool root connections
* Simplify logic: DRY, lesser indentation & all DAT * Utilize newer APIs, f-strings & more * Cleaner namespace * Conform inconsistent behaviours
* Reduce _POOL_SIZE from 4 to 1. New pools will have just one connection. They can scale up as per requirement there after. * Set auto_connect flag in MariaDB connection - https://mariadb.com/docs/connect/programming-languages/python/connect/
Codecov Report
@@ Coverage Diff @@
## develop #16961 +/- ##
===========================================
- Coverage 59.17% 57.31% -1.86%
===========================================
Files 772 771 -1
Lines 69261 69321 +60
Branches 6036 6035 -1
===========================================
- Hits 40983 39733 -1250
- Misses 24825 26099 +1274
- Partials 3453 3489 +36
Flags with carried forward coverage won't be shown. Click here to find out more. |
gavindsouza
changed the title
gavindsouza
force-pushed
the
mariadb-client-refactor
branch
2 times, most recently
from
9d12c53
to
becfa8b
Compare
This is supposed to be a temporary switch to make the parent PR easier to digest. MariaDB client has some issues with release, and system dependencies. This commit may be reverted to enable mariadb client again.
gavindsouza
force-pushed
the
mariadb-client-refactor
branch
from
27e1414
to
1a610e1
Compare
ankush
reviewed
Jul 22, 2022
This was
unlinked from
issues
Jul 22, 2022
db.default_port wil be available as a class attribute to hold defaults for DB types. Usage: frappe.conf.db_port or frappe.db.default_port Why: I couldn't run the mariadb command because the defaults aren't set for my system. server is remote / containerized. Setting port in equivalent mysql command fixes this.
There existed inconsistencies between db_query & db's fallback for min datetime in str format - missing decimal seconds places. Now, we're storing the default string once and re-using it to reduce inconsistencies or room for human errors.
Co-authored-by: Ankush Menat <ankushmenat@gmail.com>
gavindsouza
removed
the
add-test-cases
|
The last ERPNext CI build was all green, re-triggered it for confirmation. |
|
All Passing 🥳 for Frappe & ERPNext
|
This was referenced Aug 10, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Changes
Database PoolingClient change frompymysqltomariadbLazyDecode,LazyMogrifymethodsread_onlyconnections via Database classis_query_typeutilfor_updateviafrappe.get_last_doc