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

Add "raw" keyword-only parameter to Loader.get_settings() #37

Closed
wants to merge 2 commits into from

Conversation

kpinc
Copy link

@kpinc kpinc commented Apr 16, 2024

Hi,

This makes it possible to copy configurations and avoid multiple interpolations, or interpolation errors.

In a pyramid-cookiecutter-starter patch I pull out configuration from a yaml file and load it into an configparser.Config() configuration, or simply pull configuration from an ini file and load it into alembic's configuration. (No reason to write different code for yaml and ini config file sources).:
https://github.com/kpinc/pyramid-cookiecutter-starter/blob/fad9192058e950b17c3e2f0232729e69bdc4984b/%7B%7Bcookiecutter.repo_name%7D%7D/tests/sqlalchemy_conftest.py#L37
https://github.com/kpinc/pyramid-cookiecutter-starter/blob/fad9192058e950b17c3e2f0232729e69bdc4984b/%7B%7Bcookiecutter.repo_name%7D%7D/tests/sqlalchemy_conftest.py#L48

It seems like an important feature, even if pyramid-cookiecutter-starter, enhanced to support yaml config files, might somehow not need to use it.

@kpinc
Copy link
Author

kpinc commented Apr 17, 2024

Further explanation of the use of the "raw" keyword in my pyramid-cookiecutter-starter fork may be warranted.

If a yaml config file is chosen for Pyramid, and sqlalchemy is the db backend, a ini config file must still be generated for alembic. Alembic only takes an ini config file. So, when the regression tests run with a yaml config file the test fixture copies the yaml config programmaticaly and calls the alembic API to give alembic a config to use.

The convenience is that the test code need not ever worry about the format of the supplied config. It just always copies whatever config it gets as input and gives it to alembic.

The test code could be re-engineered to special-case on the config file format in-use, loading alembic's ini config specially and using 2 config files when Pyramid does not take an ini config format. But that seems clunky and opens the possibility that the 2 config files contain different configurations. This would cause confusion.

Best is to be able to cleanly copy configs, without double-interpolation/etc.

@mmerickel
Copy link
Member

mmerickel commented Apr 17, 2024

I'm trying to follow the things you're saying about this but it's against my original design of plaster to expose these types of details so having trouble wrapping my head around these cases you're describing with the cookiecutter and alembic.

If alembic needs an ini and you move to a different plaster backend that is not ini then it seems to me you just need to define a separate alembic.ini. Is that wrong? Is the proposal to do something else to convert the yaml into an alembic.Config object yourself and not have alembic load it from a file? Seems like that is what you are doing in your conftest.py at least.

It's just kind of a happy little coincidence right now that alembic is compatible with pastedeploy's ini config.

@kpinc
Copy link
Author

kpinc commented Apr 18, 2024 via email

@kpinc
Copy link
Author

kpinc commented Apr 18, 2024 via email

@kpinc
Copy link
Author

kpinc commented Apr 19, 2024 via email

@kpinc
Copy link
Author

kpinc commented Apr 21, 2024

On Wed, 17 Apr 2024 20:13:49 -0500 "Karl O. Pinc" @.***> wrote:

How does this plan sound? I'll make another fork of the cookecutter starter that does not use "raw" or otherwise require changes to plaster et-al. You can look at the two of them, diff them, etc., and decide whether any changes to plaster are warranted.

Done. There's a pyramid-cookiecutter-starter PR #137.

@kpinc
Copy link
Author

kpinc commented Apr 21, 2024

On Wed, 17 Apr 2024 20:13:49 -0500 "Karl O. Pinc" @.***> wrote:

How does this plan sound? I'll make another fork of the cookecutter starter that does not use "raw" or otherwise require changes to plaster et-al.

On reflection, I like the cookiecutter better without the "raw" parameter. It's simpler.

@kpinc
Copy link
Author

kpinc commented Aug 30, 2024

Closing this PR request. Hoping to see pyramid-cookiecutter-starter PR #137 move forward.

@kpinc kpinc closed this Aug 30, 2024
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