search

ChuloAI / BrainChulo

Harnessing the Memory Power of the Camelids
MIT License
145 stars 11 forks source link

Fix paths #51

Closed iGavroche closed 1 year ago

iGavroche commented 1 year ago

Use Docker or do not, why not?

paolorechia commented 1 year ago

I was thinking, just to be sure maybe we shouldn't use f strings. If I recall correctly it can cause issues cross-platform, and if we want to give the option not to use docker we should be wary

Really? I also used them, if it’s a problem we should update the whole code base.

paolorechia commented 1 year ago

qol thoughts on the main.py, wouldn't it be better to generate an alembic Config and updating it rather than manipulating the .ini file directly?

I only used Alembic once in a project 3 years ago, so I can't offer a solid opinion on this topic. @iGavroche I'm approving, but I also did not understand why we're generating a .ini, could you elaborate here?

iGavroche commented 1 year ago

@Karajan421 solid point on using path joining instead. I'll do that.

@paolorechia the idea is that the alembic.ini is a relative file to the current directory, and based on the direction each user goes we generate an absolute path for script_location and sys path. This way, the .ini file remains untouched and provides a proper template we can iterate on if need be.

With this said, I should say I do not consider this PR to be best practices. We should probably think of better ways to handle paths overall.