I think I would rather have the other script set DOWNLOAD_ALL_DEFAULT_REPOS=true if it needs all of them and have the default behavior of skipping unnecessary downloads.

No, that's the point. If the other scripts have to set this new DOWNLOAD_ALL_DEFAULT_REPOS=true it means I break backward-compatibility with the other scripts.

I want no changes to other scripts outside this repo.

Instead, every CI needs to inject the new variable to take advantage of more intelligent download rather than getting it for free. Since birdhouse-deploy does wget to retrieve those scripts, it makes more sense IMO that it updates with new features, or use an explicit commit hash or tag to ensure the behavior remains consistent.

every CI needs to inject the new variable to take advantage of more intelligent download

Huh, the only one is this Jenkins or more precisely the testall script. If someone deploy this job on another server, the entrypoint is still the testall script, not this downloadrepos directly.

use an explicit commit hash or tag to ensure the behavior remains consistent.

Using exact commit hash is like pinning everything in your requirements.txt. We do not do this because update will be too tedious, same here.

If the repository defined pytest options, such as necessary plugins or marker definitions, this will break their execution.

I agree this is weird. I have no other hypothesis other can because I run multiple notebooks from multiple repos at the same time, if each repo has their config, it breaks the full test run.

In that case we should be running a separate py.test process for each repo instead of all at once.

I agree with @fmigneault that there could be crucial definitions in the setup.cfg tox.ini pyproject.toml files that we do not want to lose or the tests may fail unexpectedly.

Is it possible those pytest definitions are for the unit test but here we are running notebook tests so they do not apply? Because for the current list of repos hardcoded in this Jenkins config, missing those pytest definitions are absolutely fine since forever. Keeping them are actually causing trouble.

The step to remove those files setup.cfg tox.ini pyproject.toml are not hardcoded so for external repos, they can choose to keep them if needed. See example in jenkins-params-external-repos.include.sh

Why not use a buildout/**/*.ipynb and leave the directory structure as originally defined by each repository? That would avoid all the directory renaming manipulations and potential side effects of removing pytest configs.

I previously thought that too. But actually real life usage, a flat directory is much easier to use, meaning to download the notebooks I want. In a nested layout, if I want multiple notebooks under multiple different sub-folders, I have to click a lot more. It's tiring.

Now with the option to only archive the notebooks enabled for the run, the chance to have name clash is even lower.

That would avoid all the directory renaming manipulations and potential side effects of removing pytest configs.

Not sure what you mean. I do not perform any directory renaming manipulations. The notebooks are run from the original location of the checkout because some will use relative path to get their test data in the same checkout.

I only copy the notebooks only, not the test data, to the buildout/ folder so they are archived by Jenkins, along side their matching "output" notebook so we can download both of them and diff them manually if the diff presented by Jenkins console output is impossible to understand.

As for "potential side effects of removing pytest configs", I also do not understand how that relate to Jenkins archiving config change.

All the notebooks are copied as a flat list, which could lead to conflicts between repositories. I was under the impression that the repository name was used as prefix to the archived notebook names to avoid this, but it is not the case. With the results archived this way, it is very hard to trace back the original source of the notebook.

As for "potential side effects of removing pytest configs", I also do not understand how that relate to Jenkins archiving config change.

It does not affect the archiving, but it affects how the notebooks are executed by pytest in order to generate their outputs. Therefore, if any pytest configurations options are applied to obtain certain effects needed by the tests, this could have bad side effects.

it is very hard to trace back the original source of the notebook.

I never had trouble knowing which nb is from which repo but I agree not everyone know this. Just so it is clear, it is already like this before (flat list of *.output.ipynb), I am just adding the original along-side the corresponding output.ipynb.

However, I'll add the repo name as prefix to the nb filename to help differentiate and avoid name clash.

By the way, I did preserve the already existing name clash prevention:

It does not affect the archiving

But you had your comment in this Jenkins archiving section so that's why I was mixed up.

Like I said here #138 (comment), it weird but I had to do it, otherwise the test run do not work at all. The only hypothesis I have is mentioned in that comment.

By the way, deleting the extra files is an existing behavior. I did not just add it.

This PR is maximally backward-compatible. I did not add any new processing steps other than making these processing steps available to external repos.

"look at the output in the artifact"? You mean look at the output in the "Console Output"?

No, I open the notebook directly from the artifacts page and look at its contents from the browser.

Because if you only look at the list of saved notebooks in the artifacts, you won't know which notebooks actually failed.

If I went through the artifact to look for a notebook, it's because I already know which test case I'm investigating to debug.

The listing of notebooks in the "Console Output" where any errors are also found, are using the original structure, not flat. This will give you exactly the repo and path of the notebook you want to run manually.

This is exactly why I would rather have the notebooks with the generated outputs located under the same locations as the current artifacts using the same structure as displayed in the console outputs.

This is exactly why I would rather have the notebooks with the generated outputs located under the same locations as the current artifacts using the same structure as displayed in the console outputs.

@fmigneault
Okay, how about I make all the files (original and output) keep the same folder structure as prefix. Then you have your structure to easy search for your file, I have my flat list to easily switch between multiples repos.

Something like buildout/pavics-sdi-master--docs--source--notebooks--WCS_example{,-output]}.ipynb. Good enough compromise so we can merge this?

I don't think it is a matter of compromise. The artifact structure should simply reflect the source repository. There is no need for an alternate naming convention and there is no need to define more code to handle it. The outputs should simply be saved where the notebooks are naturally collected in artifacts.

I don't think it is a matter of compromise.

Agreed, no compromise for usability.

Let me repeat comment #138 (comment) again if not clear enough: if "notebooks are under different nested folders ! You'll have to click to get to the first older, back out, then again to the 2nd folder ! The more folders, the more back and forth it is !!!"

Just a reminder, this artifact archiving just for immediate troubleshooting, it has no long term persistance value. I'd rather focus on usability and productivity than following some standard for something that is ephemeral and not important. Once you manage a lot of notebook repos, a flat list if much faster to deal with.

Even then, I do not agree. If you know the path of the notebook you are interested in after getting an error, you have no reason to go back-and-forth between folders.

I see the addition of code, special character handling and deletion of repo-specific configs as a sign of bigger maintenance burden and potential side effects.