Skip to content
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

Deprecate WorkingDirectory #1162

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

mdellweg
Copy link
Member

@mdellweg mdellweg commented Mar 3, 2021

Additionally document the use of tempfile.TemporaryDirectory.

fixes #8231
https://pulp.plan.io/issues/8231

@mdellweg mdellweg force-pushed the deprecate_working_directory branch 2 times, most recently from 3136867 to 0df6a9b Compare March 3, 2021 10:24
@pulpbot
Copy link
Member

pulpbot commented Mar 3, 2021

Attached issue: https://pulp.plan.io/issues/8231

directory. Those will automatically be cleaned up once the task is finished.

If a task needs to create more temporary directories, it is encouraged to use
``tempfile.TemporaryDirectory(dir=".")`` from the python stdandard library to place them in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo "stdandard"

If a task needs to create more temporary directories, it is encouraged to use
``tempfile.TemporaryDirectory(dir=".")`` from the python stdandard library to place them in the
tasks working directory. This is however only necessary, if the amount of temporarily saved data is
too much to wait for the automatic cleanup at the end of the task processing.
Copy link
Contributor

Choose a reason for hiding this comment

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

Or if you want to avoid file name conflicts.

@mdellweg mdellweg force-pushed the deprecate_working_directory branch from 0df6a9b to d26e040 Compare March 4, 2021 10:34
**Worker and Tasks Directories**

In pulp each worker is assigned a unique working directory living in ``/var/lib/pulp/tmp/``, and
each started task will have its own clean temporary subdirectory therin as its current working
Copy link
Member

Choose a reason for hiding this comment

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

therein maybe?

@@ -41,6 +41,6 @@ def get_policy_statements(self, request, view):
warnings.warn(
"Addressing AccessPolicy via the viewset's classname is deprecated"
"and will be removed in pulpcore==3.10; use the viewset's urlpattern().",
warnings.DeprecationWarning,
DeprecationWarning,
Copy link
Member

Choose a reason for hiding this comment

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

Should this warning be removed completely? It refers to 3.10.
It's not a part of the scope of this PR but since you are already doing some cleanup here :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we are already late here...

@@ -29,7 +29,7 @@ def get_plugin_storage_path(plugin_app_label):
"""
warnings.warn(
"get_plugin_storage_path() is deprecated and will be removed in pulpcore==3.11.",
warnings.DeprecationWarning,
DeprecationWarning,
Copy link
Member

Choose a reason for hiding this comment

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

We should remove this deprecation as well, I wonder if the get_plugin_storage_path is removed or of there is a task for it

Copy link
Member

Choose a reason for hiding this comment

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

I'll handle this one separately, FYI

Copy link
Member

Choose a reason for hiding this comment

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

@mdellweg mdellweg force-pushed the deprecate_working_directory branch from d26e040 to d35d977 Compare March 4, 2021 16:58
@ipanova
Copy link
Member

ipanova commented Mar 4, 2021

This issue is in the 3.12 milestone.Should we decide to merge it now please update the milestone accordingly.

Additionally document the use of tempfile.TemporaryDirectory.

fixes #8231
https://pulp.plan.io/issues/8231
@mdellweg mdellweg force-pushed the deprecate_working_directory branch from d35d977 to f6db3b2 Compare March 5, 2021 15:23
Copy link
Member

@goosemania goosemania left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@bmbouter bmbouter left a comment

Choose a reason for hiding this comment

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

Thank you!

@mdellweg
Copy link
Member Author

mdellweg commented Mar 5, 2021

Removing is scheduled for 3.12 https://pulp.plan.io/issues/8354

@mdellweg mdellweg merged commit 7a6a523 into pulp:master Mar 5, 2021
@mdellweg mdellweg deleted the deprecate_working_directory branch March 5, 2021 15:59
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.

None yet

6 participants