itsumura-h / nim-allographer

A query_builder/ORM library inspired by Laravel/PHP and Orator/Python for Nim
MIT License
150 stars 11 forks source link

It would be able to remove "loadConf" #25

Closed itsumura-h closed 4 years ago

itsumura-h commented 4 years ago

Not load config and write source code but use template

bung87 commented 4 years ago

the current devel branch still write connction.nim file under nimble pkgs directory, it's a bad design.

itsumura-h commented 4 years ago

@bung87 I tried but I couldn't. If it's possible, I want to do it. Is loading database.ini in compile time possible if I use macro?

bung87 commented 4 years ago

if just load file can using staticRead , but for get database connection instance just need runtime, so no need write a call func return a static file path.

bung87 commented 4 years ago

if you planed to do this, I can try this, and with https://github.com/itsumura-h/nim-allographer/issues/29

itsumura-h commented 4 years ago

@bung87 Feel free to crerate pull request! Thank you!

bung87 commented 4 years ago

I change it to runtime ,https://github.com/bung87/nim-allographer/commit/579a66c6426775e2ade9c0580849a93efcb2fa25

getCurrentDir requires runtime executing, I also tried using macro or template for dynamically import db_** , finally found it not possible, like c, c++ requires an absolute path.

now I wrapped it to DbConnection , found exec procs using singleton db connection , if the code you feel fine, I will continue modify exec.nim.

the bad part is it need compile with all db_ lib, unless we introduce a compile flag, since we programming in Non-script language, it seems fine , but need documented it make user easier to install lib

itsumura-h commented 4 years ago

@bung87

it need compile with all db_** lib

It seems that if user wants to use mysql, when compiling, postgres build error might happen because libpq is not installed.

If use compile flag, how about use $projectDir/config/nim.cfg instead of database.ini or database.yml? https://nim-lang.org/docs/nimc.html#compiler-usage-configuration-files

bung87 commented 4 years ago

https://github.com/bung87/nim-allographer/blob/develop/src/allographer/connection.nim

I work it out follow your suggestion, it will cover your current design, but the problem is the end user can only using one type database connection, may considering provider macro to end user, let the end user choose. eg. modify decDbConnection accept params return conn directly.

the second considering: could only provider macro to initialize connection, let the configuration part left to framework side. the yaml lib can't load yaml at compile time, so the framework side also need do more research.

itsumura-h commented 4 years ago

@bung87 I cloned your source code and it works! You need some bug fix, however combination of NimScript and macro is wonderhul. I successfully run example/run.nim

connection.nim

newLit(HOST),
newLit(USERNAME),
newLit(PASSWORD),
newLit(DATABASE),

Be careful the order. DATABASE should be placed last.

bung87 commented 4 years ago

@itsumura-h ok.I got it , so what's your thoughts about design , to support multi type db connection , if you willing to do this ,may causes make a large change to current code, I may dig further to implement a adapter like other lib or framework does. glad atleast we have one solution now.

itsumura-h commented 4 years ago

@bung87 Does implement a adapter means that config/database.yml can be used? If it's possible, it's greater than using nimscript and env variable.

bung87 commented 4 years ago

problem 1: for support database.yml need parse yml at compile time, since we use macro to fetch configuration. the lib yaml doest support yet. even this done, it only support end user using one type database connection, well it fit the database.yml use case , since it only need one type database connection . problem 2: I am considering , the end user may have multiple type of database connection , eg db1(sqlite) do something db2(mysql) do something

so I'll try fix problem 2 , so the end user can switch db in their procs.

itsumura-h commented 4 years ago

@bung87 Are db1 and db2 proc? Is changing database possible by one part?

By the way, please rebase from latest develop branch

bung87 commented 4 years ago

just demonstrate the end user use case , not actually code , sorry about my poor english. I'm 60 % sure it's possible. just counts on myself. anyway I'll give a try.

itsumura-h commented 4 years ago

@bung87 I could dynamic importing by using macro refer to your code. No need to run "loadConf" command. Please check it out.

https://github.com/itsumura-h/nim-allographer/blob/Feature25/remove_loadConf/src/allographer/connection.nim

https://github.com/itsumura-h/nim-allographer/blob/Feature25/remove_loadConf/tests/conf/database.nim

itsumura-h commented 4 years ago

I use config.nims and env variables because when entry-point file is different, getProjectPath() return different config path If using projectDir/config.nims, it is available in child dir.