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

Custom path to DB connection Details ignored #641

Closed
circulon opened this issue Apr 23, 2022 · 6 comments
Closed

Custom path to DB connection Details ignored #641

circulon opened this issue Apr 23, 2022 · 6 comments
Labels
bug An existing feature is not working as intended

Comments

@circulon
Copy link
Contributor

Describe the bug
Using custom connection details providing a non default path to database config file are ignored.

To Reproduce

i ran into this while playing with a custom Masonite config
But this will also be the case if others use a non standard location for their config

Steps to reproduce the behavior:

  1. setup basic Masonite App
  2. make a new module called "core" inside the "config" module
  3. move all the .py files from "config" module into "config.core" module
  4. edit the Kernel.py file
  5. in the register_configurations method change
    self.application.bind("config.location", "config")
    to
    self.application.bind("config.location", "config.core")
  6. run the local server
  7. see the error
    masoniteorm.exceptions.ConfigurationNotFound: ORM configuration file has not been found in config.database.

Expected behavior
the custom config path should be used to load the config file

Screenshots or code snippets
N/A

Desktop (please complete the following information):

  • OS: Mac OSX
  • Version: 10.15.7

What database are you using?

  • Type: Postgres
  • Version 10.5
  • Masonite ORM: latest

Additional context
N/A

@circulon circulon added the bug An existing feature is not working as intended label Apr 23, 2022
@circulon circulon changed the title Custom connection Details ignored Custom path to DB connection Details Apr 23, 2022
@circulon circulon changed the title Custom path to DB connection Details Custom path to DB connection Details ignored Apr 23, 2022
@girardinsamuel
Copy link
Contributor

To solve this you should normally redefine the DB_CONFIG_PATH environment variable in your .env file to:

DB_CONFIG_PATH=config/core/database

@josephmancuso
Copy link
Member

Yea this is the solution 👆

@girardinsamuel is this not documented? I don't see it in the install part of Masonite orm

@circulon
Copy link
Contributor Author

To solve this you should normally redefine the DB_CONFIG_PATH environment variable in your .env file to:

DB_CONFIG_PATH=config/core/database

This is fine if your using .env files which I am not.

To clarify I am building a webapp in the cloud (AWS Lambda)
All environment variable on a given Lambda are visible when the lambda is viewed in the console.

This is obviously not desirable for sensite items like passwords and external api keys (eg Stripe)

In my case the config files all pull their item info from AWS Parameter Store (SSM) and so is only available to the code at runtime.

my desired config folder structure looks like this:

config

  • core
    • application.py
    • database.py
    • cache.py
    • ...
  • environment
    • dev
      • application.py
      • database.py
      • cache.py
      • ...
    • staging
      • application.py
      • ...
    • prod
      • ...
    • deploy
      • ....

The config gets loaded in the following order and each layer has the ability to override the setting if set previously.

  • config/core
  • config/
  • config/deploy

This creates a hierarchy that is obvious IMHO and very modular.

If i have to have the database.py on its own (ie not with the other "core" modules, this breaks the Masonite configuration loader in a really bad way as well as being confusing to other developers about the reason for file placement outside of its scope of use (being the folder its in).

So back to my original point with the config path.
As the load_config() function accepts a path and that path is used as a priority why is it not used in all places?

The custom path is used in some classes eg ShellCommand and Migration so why not in the core of the ORM?
By having this ability it provides a more flexible usage option.

I hope this clarifies things for you.

PS my PR #642 is not passing tests (not sure why) but you can see the additions I have made so far to demonstrate what I am explaining here.

@josephmancuso
Copy link
Member

@girardinsamuel do you think we can optionally load the module in directly somehow instead of pointing a string to the module location?

That would allow us to be have more flexibility over how it loads in then

@girardinsamuel
Copy link
Contributor

girardinsamuel commented Jun 3, 2022

do you think we can optionally load the module in directly somehow

I am not sure I understand what you mean 🤔

@circulon
Copy link
Contributor Author

circulon commented Jun 3, 2022

@girardinsamuel @josephmancuso
This is not a major issue as I am now setting the DB_CONFIG_PATH env var (as you recommended) internally in my app, so it works and the env var is not exposed at the OS level.

That said this is still an issue that should still be addressed at some stage, though imm not sure how to fully do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing feature is not working as intended
Projects
None yet
Development

No branches or pull requests

3 participants