[PR #124] [CLOSED] General Refactoring #156

New issue

Closed

opened 2026-02-26 01:33:24 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 01:33:24 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/jeffknupp/sandman2/pull/124
Author: @Carelvd
Created: 9/18/2019
Status: ❌ Closed

Base: master ← Head: master

📝 Commits (10+)

0acd441 Renamed the main package
3754fde Exception registration moved
1b4ea99 Database
4cd2c1f Administration
806646a Moved service registration
e17fc70 Moved model registration
c7332ff Cleaned up the application factory
cca3c2e Cleaned up the api
d903fcf Neatened up the API
cf4f799 Modified the application function

📊 Changes

38 files changed (+531 additions, -402 deletions)

View changed files

📝 .travis.yml (+1 -1)
📝 dockerfiles/start.sh (+1 -1)
📝 docs/conf.py (+6 -6)
➕ docs/errors.rst (+11 -0)
📝 examples/example_automap.py (+2 -2)
📝 examples/example_user_models.py (+2 -2)
📝 examples/user_models.py (+1 -1)
➕ flask_sandman/__init__.py (+5 -0)
➕ flask_sandman/__main__.py (+68 -0)
➕ flask_sandman/admin.py (+36 -0)
➕ flask_sandman/api.py (+50 -0)
➕ flask_sandman/app.py (+47 -0)
➕ flask_sandman/database.py (+12 -0)
📝 flask_sandman/decorators.py (+3 -5)
📝 flask_sandman/exception.py (+46 -14)
📝 flask_sandman/model.py (+79 -17)
📝 flask_sandman/service.py (+98 -44)
📝 flask_sandman/templates/admin/index.html (+0 -0)
📝 flask_sandman/templates/create.html (+0 -0)
📝 flask_sandman/templates/edit.html (+0 -0)

...and 18 more files

📄 Description

Dear Mr. Knupp,

I was hoping to make this PR against a development branch in your repository but it seems that Github does not allow for this and forces one to push to an existing branch, so I went with the master branch again, I trust this is fine with you.

I have tried to follow the Flask Extension/Pallets Project guidelines with this PR. The major changes are as follows :

Major refactorings are as follows :
- Moving the functions within app.py into their "parent" modules e.g. register_service is now the register function in service.py, similarly there are register functions in model.py, views.py, exception.py and admin.py.
- Splitting of the logic in flask_sandman.app:application into flask_sandman.api:sandman so that there is a central function any user of the library can invoke within their own create_app methods. I have also refactored this into a Sandman class akin to SQLAlchemy or Api from flask restful but it's rough and omitted from this PR.
- Combining _register_user_models and _reflect_all into flask_sandman.api:sandman removing the need for the reflect_all flag. This was a touch more intricate then I initially realized.
- Within flask_sandman.app:application there is a "router" variable this is assigned either the application or, if one is provided, a blueprint. I am developing an application against this feature and haven't had any trouble when using a blueprint so far. I haven't had the chance to port my experiences to uni test in Sandman just yet though.
Renaming of the package components to conform to the "standard" structure it seems Palette encourages :
- Renaming the base package from Sandman2 to flask_sandman.
- Renaming get_app to create_app; internally this is called application but it is exposed to the user as create_app e.g. from flask_sandman import create_app.

I have also done some work on the documentation. This is still a bit rough though and I thought it best to make another PR's if this one is accepted.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/jeffknupp/sandman2/pull/124 **Author:** [@Carelvd](https://github.com/Carelvd) **Created:** 9/18/2019 **Status:** ❌ Closed **Base:** `master` ← **Head:** `master` --- ### 📝 Commits (10+) - [`0acd441`](https://github.com/jeffknupp/sandman2/commit/0acd441e260073c98598b7097136de5029383cf1) Renamed the main package - [`3754fde`](https://github.com/jeffknupp/sandman2/commit/3754fde3ae661c3a42f8df4afb55a6e6e3f4df81) Exception registration moved - [`1b4ea99`](https://github.com/jeffknupp/sandman2/commit/1b4ea9989254f8260b42ad2d252316a2b63fc850) Database - [`4cd2c1f`](https://github.com/jeffknupp/sandman2/commit/4cd2c1f754bf8436db1b3c35863b1d121da7ae17) Administration - [`806646a`](https://github.com/jeffknupp/sandman2/commit/806646a48943b8d18f0d8d7ca4ac86c21f157693) Moved service registration - [`e17fc70`](https://github.com/jeffknupp/sandman2/commit/e17fc708045e196d3d329e71e07f6acdfa780b86) Moved model registration - [`c7332ff`](https://github.com/jeffknupp/sandman2/commit/c7332ff51fe540b5ce421dd38eda033df7829029) Cleaned up the application factory - [`cca3c2e`](https://github.com/jeffknupp/sandman2/commit/cca3c2ee75d541ddea82b69fce73fe7b4c13cc16) Cleaned up the api - [`d903fcf`](https://github.com/jeffknupp/sandman2/commit/d903fcf0450bcd40d5e6ee9e88f873b4f284ef92) Neatened up the API - [`cf4f799`](https://github.com/jeffknupp/sandman2/commit/cf4f79993ba8aa4575d6d64af8fc3051d0b689b9) Modified the application function ### 📊 Changes **38 files changed** (+531 additions, -402 deletions) <details> <summary>View changed files</summary> 📝 `.travis.yml` (+1 -1) 📝 `dockerfiles/start.sh` (+1 -1) 📝 `docs/conf.py` (+6 -6) ➕ `docs/errors.rst` (+11 -0) 📝 `examples/example_automap.py` (+2 -2) 📝 `examples/example_user_models.py` (+2 -2) 📝 `examples/user_models.py` (+1 -1) ➕ `flask_sandman/__init__.py` (+5 -0) ➕ `flask_sandman/__main__.py` (+68 -0) ➕ `flask_sandman/admin.py` (+36 -0) ➕ `flask_sandman/api.py` (+50 -0) ➕ `flask_sandman/app.py` (+47 -0) ➕ `flask_sandman/database.py` (+12 -0) 📝 `flask_sandman/decorators.py` (+3 -5) 📝 `flask_sandman/exception.py` (+46 -14) 📝 `flask_sandman/model.py` (+79 -17) 📝 `flask_sandman/service.py` (+98 -44) 📝 `flask_sandman/templates/admin/index.html` (+0 -0) 📝 `flask_sandman/templates/create.html` (+0 -0) 📝 `flask_sandman/templates/edit.html` (+0 -0) _...and 18 more files_ </details> ### 📄 Description Dear Mr. Knupp, I was hoping to make this PR against a `development` branch in your repository but it seems that Github does not allow for this and forces one to push to an existing branch, so I went with the master branch again, I trust this is fine with you. I have tried to follow the Flask Extension/Pallets Project [guidelines](https://flask.palletsprojects.com/en/1.1.x/extensiondev/#approved-extensions) with this PR. The major changes are as follows : * Major refactorings are as follows : * Moving the functions within `app.py` into their "parent" modules e.g. `register_service` is now the `register` function in `service.py`, similarly there are register functions in `model.py`, `views.py`, `exception.py` and `admin.py`. * Splitting of the logic in `flask_sandman.app:application` into `flask_sandman.api:sandman` so that there is a central function any user of the library can invoke within their own `create_app` methods. I have also refactored this into a `Sandman` class akin to `SQLAlchemy` or `Api` from flask restful but it's rough and omitted from this PR. * Combining `_register_user_models` and `_reflect_all` into `flask_sandman.api:sandman` removing the need for the `reflect_all` flag. This was a touch more intricate then I initially realized. * Within `flask_sandman.app:application` there is a "router" variable this is assigned either the application or, if one is provided, a blueprint. I am developing an application against this feature and haven't had any trouble when using a blueprint so far. I haven't had the chance to port my experiences to uni test in Sandman just yet though. * Renaming of the package components to conform to the "standard" structure it seems Palette encourages : * Renaming the base package from `Sandman2` to `flask_sandman`. * Renaming `get_app` to `create_app`; internally this is called `application` but it is exposed to the user as `create_app` e.g. `from flask_sandman import create_app`. I have also done some work on the documentation. This is still a bit rough though and I thought it best to make another PR's if this one is accepted. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>