Skip to content

[PVC][chore] remove contentDescriptorToLayer in favor of contentDescriptorToLayerPVC #9496

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

Closed
Tracked by #7901
sagor999 opened this issue Apr 22, 2022 · 4 comments
Closed
Tracked by #7901
Labels
meta: stale This issue/PR is stale and will be closed soon team: workspace Issue belongs to the Workspace team

Comments

@sagor999
Copy link
Contributor

After this PR gets merged, we should harmonize contentDescriptorToLayerPVC and contentDescriptorToLayer by switching to using /.workspace' folder to place content.json file, instead of placing it into /workspace` folder, and thus removing a separate code path for PVC implementation.
For more info: #9242 (comment)

@sagor999 sagor999 added the team: workspace Issue belongs to the Workspace team label Apr 22, 2022
@sagor999 sagor999 self-assigned this Apr 22, 2022
@sagor999 sagor999 moved this to In Progress in 🌌 Workspace Team May 19, 2022
@sagor999
Copy link
Contributor Author

This doesn't quite work for old way of doing things, since ws-daemon attempts to place content.json into /workspace folder:

func RunInitializer(ctx context.Context, destination string, initializer *csapi.WorkspaceInitializer, remoteContent map[string]storage.DownloadInfo, opts RunInitializerOpts) (err error) {

It is not clear how easy it would be to change that function, and I don't think it will be worth the effort (and potential unaccounted edge cases)
So I think it is best to just remove old contentDescriptorToLayer once we 100% switch to PVC.

@stale
Copy link

stale bot commented Sep 24, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Sep 24, 2022
@sagor999 sagor999 changed the title harmonize contentDescriptorToLayerPVC and contentDescriptorToLayer [PVC][chore] remove contentDescriptorToLayer in favor of contentDescriptorToLayerPVC Sep 26, 2022
@sagor999
Copy link
Contributor Author

Changed title of the issue.
Once PVC is fully working, remove old code path instead of trying to merge those two together in the meantime.

@sagor999 sagor999 removed their assignment Sep 26, 2022
@sagor999 sagor999 removed the meta: stale This issue/PR is stale and will be closed soon label Sep 26, 2022
@stale
Copy link

stale bot commented Jan 2, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Jan 2, 2023
@stale stale bot closed this as completed Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta: stale This issue/PR is stale and will be closed soon team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

No branches or pull requests

1 participant