fix(external_script): fix crash during concurrent cleanup

[Thibault-Pelletier]

Fix crash in external script cleanup when application is run in multi concurrent process.

[bourdaisj]

Thanks for the fix, looks very nice at first glance

[bourdaisj]

I had a feeling about it, and after testing I confirm this breaks the docker image build assets collection. We might to think of a better way to handle this. The problem is that trame.tools.www expects to find the static assets, but since that change make it that the module cleans after itself, the assets are not here.

[bourdaisj]

let me think about it

[bourdaisj]

any idea maybe @jourdain ?

[bourdaisj]

A simple solution would be to keep (partially) the old behavior so that the assets are placed at the correct location (user_provided_scripts folder) - but we use the new tmp dir system to serve the assets when running in local.
In a "real" deployment with docker the copy mechanism does not help because the assets are directly served the the web server, entirely bypassing trame.
I think the issue you are fixing with this PR only happens in the situation you encountered with trame-slicer when you spawn multiple processes of the same app without using the centralized collected assets folder - typically /deploy/server/www in docker container.

Another solution could be to have some kind of "webserver" mode we could use to tell trame-client (and any other trame package...) that the assets are being taken care of by the web server and there's nothing to do in the module init. (besides the server.enable_module call). But I'm not sure I like it because AFAIK, the external script feature is the only one that allow library consumers to register custom scripts to be served directly by the web server. Well you can do it manually, but in this case it is not trame's job to make sure everything is being cleared/served properly

[bourdaisj]

From a discussion with Thibault:
Having an environment variable we set by default in the base docker image to tell trame-client (and any other packages) that they do not need to copy/cleanup any assets (since being taken care of by apache and collected ahead of time in the image build process) could heavily make sense.
We also discussed that the current process for asset collection isn't intuitive since you need to modify the docker initialize.sh script.
I personnally that you make sense to modify the default initialize.sh:

• add some set -euxo pipefail so that any failed command makes the whle thing fail
• always run the app entrypoint python /deploy/server/venv/bin/ --timeout 1 --server or at least find a way to automatically collect the assets without requiring the user to change anything to the build process.

[jourdain]

Technically, we already have a cli flag --no-http that you may leverage... And then in docker, you need to make sure you leverage it.

[Thibault-Pelletier]

After discussing with @bourdaisj, and following discussions in this thread, this PR is not going in the right direction to reliably serve custom JS files / fix the concurrency issue.

I'll be closing it and we will keep on iterating on the approach with other trame developers.

[bourdaisj]

Thanks @jourdain ; I forgot about that one. We should definitely use it for those kind of use-case.
Additionally we discussed with Thibault and mentioned two solutiosn for the automatic asset collection:

• declarative approach using the setup/apps.yml file. users could declare their handler static assets here
• based on convention - we could have a dedicated standardized folder for containing such assets that we would add to the cookiecutter