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

FIX: Mengikat model ke pengkalan data untuk aplikasi pelayan #22

Merged
merged 3 commits into from
Mar 28, 2023

Conversation

mshumayl
Copy link
Contributor

@mshumayl mshumayl commented Nov 22, 2022

Isu yang dituju

#20 #21

Perubahan yang dibuat

  • Mengikat (bind) model dan pengkalan data melalui fungsi create_tables daripada samudra.models.

Perincian perubahan lain

  • Mengalihkan pemalar global ENGINE, DATABASE_NAME ke samudra/server/setup.py untuk mengelakkan nilai ini dirujuk sebelum pentakrifan semasa proses permulaan.
  • Meletakkan pemalar global PATH di dalam samudra/server/setup.py untuk menjelaskan keperluan argumen path dalam panggilan get_database di samudra/server/setup.py.
  • Menukarkan argumen reload kepada False untuk panggilan kaedah uvicorn.run() untuk mengelakkan proses pemulaan, penciptaan, dan pengikatan pengkalan data diulangi ketika pelayan sedang berjalan.

Kalau boleh, minta tolong @Thaza-Kun tolong semak sama ada perubahan ini masuk akal ataupun ada perkara yang boleh dibuat dengan lebih molek.

@mshumayl mshumayl changed the title FIX: Bind model to database for server app #20 #21 FIX: Bind model to database for server app Nov 22, 2022
@mshumayl mshumayl changed the title FIX: Bind model to database for server app FIX: Mengikat model ke database untuk aplikasi pelayan Nov 23, 2022
@mshumayl mshumayl changed the title FIX: Mengikat model ke database untuk aplikasi pelayan FIX: Mengikat model ke pengkalan data untuk aplikasi pelayan Nov 23, 2022
@Thaza-Kun
Copy link
Collaborator

Aku dah jelaskan satu bahagian kod dan buang bahagian yang rasa tak patut.

if new:
    # If a new database is to be created, check if the given path is occupied.
    # If not occupied, create the database.
    # If occupied, raise FileExistsError
    try:
        base_path.mkdir(parents=True)
    except FileExistsError:
            print(f"Populating empty folder `{base_path.resolve()}` with {db_file}")
        else:
            raise FileExistsError(
                f"The path `{base_path.resolve()}` is already occupied with something else. Try passing `new=False` to access the database or consider creating new database in another folder."
            )
    # Set up readme
    README = Path(base_path, "README.md")
                "Created using [samudra](https://github.com/samudradev/samudra)",
            ]
        )
# else:
#     # Originally, this part was intended to get the path to database via its name in the local `~/.samudra/databases.toml` file when `new=False`.
#     # However, that would mean that the `path` parameter is rendered meaningless unless `new=True`.
#     # Perhaps this functionality should be outside the function with paths and folder parameters as its result
#     # which will be passed to `get_database/get_sqlite` with explicit `new=False`
#     db_obj = get_database_info(name=db_file)
#     if db_obj is None:
#         return FileNotFoundError(
#             f"The database name {db_file} is not found. Perhaps it is not created yet. Pass the key `new=True` if that's the case"
#         )
#     base_path: Path = Path(db_obj["path"], folder=db_file)
#     full_path: Path = Path(base_path, db_file)

Sepatutnya tidak ada masalah kalau new=True dibuat sekali sahaja (kalau baru nak buat db) kemudian new=False untuk seterusnya bila dah ada db kat path yang sepatutnya.

@mshumayl
Copy link
Contributor Author

Aku fikir nak tambah beberapa test cases untuk ini kalau boleh.

Jadi sebelum tu, aku kena tanya berkenaan #23. Ada dua cara yang aku cadangkan untuk menyelesaikan ralat ImportError ketika menjalankan pytest untuk modul yang ada import relatif. Kalau boleh nak minta pendapat @Thaza-Kun, pendekatan mana yang lebih sesuai untuk projek ini.

@Thaza-Kun
Copy link
Collaborator

Aku baru tahu peewee ada sediakan class DatabaseProxy untuk memudahkan database yang pelbagai. Oleh itu, aku ada buat beberapa perubahan:

  1. Letak database = DatabaseProxy dalam BaseModel
  2. Asingkan proses create, get, set_active, dan get_active untuk database supaya tugasnya tidak lagi bercampur aduk dalam satu fungsi.

@Thaza-Kun
Copy link
Collaborator

Aku fikir nak tambah beberapa test cases untuk ini kalau boleh.

Jadi sebelum tu, aku kena tanya berkenaan #23. Ada dua cara yang aku cadangkan untuk menyelesaikan ralat ImportError ketika menjalankan pytest untuk modul yang ada import relatif. Kalau boleh nak minta pendapat @Thaza-Kun, pendekatan mana yang lebih sesuai untuk projek ini.

Aku sangat hargai kalau dapat buat test. Aku ada jugak cuba buat test bahagian server tapi asyik gagal bab routing.

@mshumayl
Copy link
Contributor Author

mshumayl commented Dec 3, 2022

Baik, aku akan mula cuba buat test untuk proses pemulaan/pengikatan database dahulu ya.

@Thaza-Kun
Copy link
Collaborator

Salam @mshumayl ada kemajuan di sini?

@Thaza-Kun Thaza-Kun merged commit 877cf21 into samudradev:master Mar 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants