Skip to content

Adding script name to 'global_config' during application setup #3735

Merged
mmerickel merged 8 commits intoPylons:mainfrom
adroullier:script_info
Jan 28, 2024
Merged

Adding script name to 'global_config' during application setup #3735
mmerickel merged 8 commits intoPylons:mainfrom
adroullier:script_info

Conversation

@adroullier
Copy link
Copy Markdown
Contributor

This code extends pyramid startup scripts in python/bin directory (pserve, prequest, pshell, pviews, proutes, ptweens) with additional info about the calling script. The additional info is passed to the **main(global_config, settings) function as part of global_config dict during wsgi setup as 'script'.
global_config can contain custom values loaded from the ini file, so to avoid unwanted overwriting of a existing 'script' value I've added a extra condition for comapatibility.

In some cases it makes sense to call different code whether the application is created as server (bin/pserve) or for a single request (bin/prequest). For example setting up internal caching modules, maintenance or other background functions.

@adroullier adroullier marked this pull request as draft October 24, 2023 12:22
@adroullier adroullier marked this pull request as ready for review October 24, 2023 13:01
Copy link
Copy Markdown
Member

@mmerickel mmerickel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. could you please tweak these to use config_vars.setdefault('__script__', 'pshell') as a simpler one-liner?
  2. Is there a way to make this work with pyramid.paster.bootstrap as well, or only the scripts? I believe this use case is not addressed here and was the topic of the recent mailing list posts triggering this work. I'm thinking __script__ == 'bootstrap' in that case.

@adroullier
Copy link
Copy Markdown
Contributor Author

1: no problem.

2: pyramid.paster.bootstrap is called in the scripts proutes, pshell, ptweens, pviews. In this case the script name is passed to bootstrap() as part of the options parameter (which is then passed to the callback get_app()). If bootstrap() gets called from other parts of the code there is no such info.
You are right, it makes sense to add info to boostrap(), too. Maybe '__script__'='bootstrap' could be used for all these scripts (proutes, pshell, ptweens, pviews). It is more consistent.

@mmerickel
Copy link
Copy Markdown
Member

mmerickel commented Oct 26, 2023

It could be the default value going back to your logic that won’t set it if already defined. Then individual scripts can still override it as you’re doing?

@adroullier
Copy link
Copy Markdown
Contributor Author

The scripts (except pserve, prequest) call pyramid.paster.bootstrap, so the script name is set before bootstrap() is called and shouldn't be a problem.

I had a look at the email discussion and pyramid code again. The original question was if there is way to have more infos in the context of the ApplicationCreated event. The event is triggered in pyramid.config.Configurator.make_wsgi_app(). The PR does not change this behaviour and I don't think there is a way to change the event.

@mmerickel
Copy link
Copy Markdown
Member

The user would need to forward the setting from global _config into their app but at least it’s an automatic setting versus one they have to set/maintain in their config files.

@mmerickel mmerickel self-requested a review October 30, 2023 20:58
Copy link
Copy Markdown
Member

@mmerickel mmerickel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the status of this from your perspective @adroullier? Trying to understand if you think this is done or if there's still a plan to handle the bootstrap case. I still think we should define __script__ if bootstrap is used directly, not just via these p* scripts.

Rest of the PR looks great and I'd be happy to merge it.

@adroullier
Copy link
Copy Markdown
Contributor Author

Sorry I haven't mentioned it in my last comment.
Actually I haven't found a use case or point where custom pyramid code is called after calling pyramid.paster.bootstrap() (and I think that is what you are referring to?).
If bootstrap()is called by custom code the developer knows the environment anyway before calling bootstrap(). So adding the script name there means just adding a few lines of unnecessary code.
Or is there a mistake in my argumentation?

I can also add the code to set the name to 'bootstrap', like

pyramid.paster.bootstrap() line 127

def bootstrap(config_uri, request=None, options=None):
    # ...
    if options is None:
        options = dict()
    options.setdefault('__script__', 'bootstrap')
    app = get_app(config_uri, options=options)
    env = prepare(request)
    env['app'] = app
    return env

@mmerickel
Copy link
Copy Markdown
Member

mmerickel commented Dec 1, 2023

Well the goal is to still add that so the app can differentiate the scenario from a normal startup using that __script__ protocl without relying on every user setting it themselves prior to calling bootstrap. I think it's worth adding that in the spirit of this PR.

@mmerickel
Copy link
Copy Markdown
Member

Fixes #3734.

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