Skip to content

Handle keep-ipynb correctly now that fileInformationCache is responsible for the cleaning #12793

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

Merged
merged 8 commits into from
May 23, 2025

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented May 21, 2025

This is a first proposal to fix #12780

It is based on the fact that

So this PR does now in execute() set transient: false to prevent cleaning when keep-ipynb: true

The cleaning is still done by fileInformationCache cleanup step.

To ensure that happens, the safeCloneDeep escape mode is leveraged to ensure that the project context fileInformationCache will always be accessible by reference.

I said first proposal because there is supposed to be a cleaning mechanism in engine with
executeTargetSkipped and so I don't see why #12337 was requiring this fileInformationCache cleanup.

@posit-snyk-bot
Copy link
Collaborator

posit-snyk-bot commented May 21, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@cderv cderv marked this pull request as ready for review May 23, 2025 09:38
@cderv cderv force-pushed the fix/keep-ipynb branch from 0bb9f09 to f553f90 Compare May 23, 2025 09:39
cderv pushed a commit that referenced this pull request May 23, 2025
backport of #12793

- update fileInformationCache based on `keep-ipynb` information
- requires to always pass ProjectContext in ExecuteOptions
- requires to ensure fileInformationCache is not cloned by safeCloneDeep
  This allows us to always get the global fileInformationCache from the project context to get / set information in it from different contexts.
- Add tests for project context and single file context
cderv pushed a commit that referenced this pull request May 23, 2025
backport of #12793

- update fileInformationCache based on `keep-ipynb` information
- requires to always pass ProjectContext in ExecuteOptions
- requires to ensure fileInformationCache is not cloned by safeCloneDeep
  This allows us to always get the global fileInformationCache from the project context to get / set information in it from different contexts.
- Add tests for project context and single file context
cderv added a commit that referenced this pull request May 23, 2025
backport of #12793

- update fileInformationCache based on `keep-ipynb` information
- requires to always pass ProjectContext in ExecuteOptions
- requires to ensure fileInformationCache is not cloned by safeCloneDeep
  This allows us to always get the global fileInformationCache from the project context to get / set information in it from different contexts.
- Add tests for project context and single file context
cderv added a commit that referenced this pull request May 23, 2025
backport of #12793

- update fileInformationCache based on `keep-ipynb` information
- requires to always pass ProjectContext in ExecuteOptions
- requires to ensure fileInformationCache is not cloned by safeCloneDeep
  This allows us to always get the global fileInformationCache from the project context to get / set information in it from different contexts.
- Add tests for project context and single file context
@cderv cderv added the backport label May 23, 2025
@cderv cderv added this to the Hot-fix milestone May 23, 2025
@cderv cderv merged commit dd24d1f into main May 23, 2025
49 checks passed
@cderv cderv deleted the fix/keep-ipynb branch May 23, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keep-ipynb does not keep notebook
2 participants