Project

Profile

Help

Task #1488

closed

Deprecate nodes

Added by bmbouter about 7 years ago. Updated almost 4 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Assignee:
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Platform Release:
2.12.0
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
Pulp 2
Sprint:
Sprint 12
Quarter:

Description

Based on multiple discussions nodes support is being deprecated with 2.8.0. As such we need to take a few steps to properly deprecate it. See the checklist for details. The nodes docs they refer to are here[0].

NOTE: with DeprecationWarning we need to ensure they are having the effect we think they are. Two things to consider/test:

  • In Python 2.6 DeprecationWarning was loud by default, but has been silenced in 2.7+.
  • The -W argument (iirc) enables/disables warnings so that could be a meaningful difference between a dev and production environment

[0]: http://pulp.readthedocs.org/en/latest/user-guide/nodes.html


Related issues

Blocks Pulp - Task #2460: Write a blog post about nodes being deprecated and what functionality users can use to replace themCLOSED - CURRENTRELEASEdaviddavis

Actions
Actions #1

Updated by mhrivnak about 7 years ago

  • Priority changed from High to Normal
  • Severity changed from 3. High to 2. Medium
  • Platform Release deleted (2.8.0)
  • Triaged changed from No to Yes
Actions #2

Updated by mhrivnak about 7 years ago

  • Tracker changed from Issue to Task
  • Groomed set to Yes
  • Sprint Candidate set to Yes
Actions #3

Updated by mhrivnak over 6 years ago

  • Sprint Candidate changed from Yes to No
Actions #4

Updated by amacdona@redhat.com over 6 years ago

  • Sprint Candidate changed from No to Yes
Actions #5

Updated by mhrivnak over 6 years ago

  • Sprint/Milestone set to 27
Actions #6

Updated by jortel@redhat.com over 6 years ago

  • Sprint/Milestone changed from 27 to 28
Actions #7

Updated by daviddavis about 6 years ago

  • Assignee set to daviddavis
Actions #8

Updated by ipanova@redhat.com about 6 years ago

  • Assignee deleted (daviddavis)
  • Sprint/Milestone changed from 28 to 29
Actions #9

Updated by daviddavis about 6 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to daviddavis
Actions #10

Updated by daviddavis about 6 years ago

Actions #11

Updated by daviddavis about 6 years ago

Actions #12

Updated by daviddavis about 6 years ago

Actions #13

Updated by daviddavis about 6 years ago

Actions #14

Updated by daviddavis about 6 years ago

Actions #15

Updated by daviddavis about 6 years ago

Actions #16

Updated by daviddavis about 6 years ago

Actions #17

Updated by daviddavis about 6 years ago

What should I do about node calls that rely on code that's not specific to nodes? For example, when I call node sync on the parent, it looks like the server just creates a consumer update_content task[0]. I may be mistaken but it looks like this task isn't specific to nodes (at least for the parent—on the child, there is some node specific importer code).

I could either:

1. Add code to pulp core and check if the consumer is a node and then display a warning message
2. Not worry about showing a deprecation warning when code not specific to nodes (but used by nodes) is called.

I'm kind of leaning toward option 2 as I had hoped to keep this change limited to nodes directory.

[0] https://git.io/v1IHM

Actions #18

Updated by daviddavis about 6 years ago

As mentioned in the bug description, it does indeed look like DeprecationWarning is silenced by default in 2.7. One option would be to run python with '-W all' or something similar in dev mode. I'm not sure exactly where/how to configure this flag though?

Also, we could update the logging code[0] by adding something like:

warnings.simplefilter('always', DeprecationWarning)

I'm not sure how to tell though when we're in a dev setup. Maybe we could add a deprecation config variable?

[0] https://github.com/pulp/pulp/blob/91a1e28c9e7d3dee418d5c7680dbf25c3e7adc63/server/pulp/server/logs.py

Actions #19

Updated by bmbouter about 6 years ago

I think we should not add the -W and instead use the warnings.simplefilter as you suggest. Adding that to the logging setup code would set that for all server side processes, but you'll also have to add it for pulp-admin and pulp-consumer. Those standalone, client-side binaries don't use the logging code.

In terms of the shared codepaths, I also prefer option 2 where we only add deprecation warnings to code that we know is node specific. Overall I believe for each server side processes (daemon processes) having at least 1 deprecation warning logged for each node operation (sync, publish) would be good. Also for interactive processes (pulp-admin, pulp-consumer) having at least 1 deprecation warning shown in the foreground for each node operation would be good.

Actions #20

Updated by daviddavis about 6 years ago

Actions #21

Updated by daviddavis about 6 years ago

  • Blocks Task #2460: Write a blog post about nodes being deprecated and what functionality users can use to replace them added
Actions #22

Updated by daviddavis about 6 years ago

Went ahead and opened a separate issue to address creating a blog post after this issue is closed out:

https://pulp.plan.io/issues/2460

Actions #23

Updated by daviddavis about 6 years ago

bmbouter, you mentioned that we previously deprecated nodes in a release of pulp but looking through the docs/user-guide/release-notes directory of pulp, I am not seeing it. Do you remember the release?

Actions #24

Updated by daviddavis about 6 years ago

So turning on logging for deprecation warnings is spitting out tons of deprecation warnings in places we don't control like:

Dec 02 09:06:35 dev.example.com pulp[18247]: py.warnings:WARNING: (18247-87552) /usr/lib/python2.7/site-packages/mongoengine/queryset/base.py:461: DeprecationWarning: update is deprecated. Use replace_one, update_one or update_many instead.
Dec 02 09:06:35 dev.example.com pulp[18247]: py.warnings:WARNING: (18247-87552)   upsert=upsert, **write_concern)

This is from mongoengine calling update which pymongo deprecated: https://github.com/mongodb/mongo-python-driver/blob/master/pymongo/collection.py#L2482-L2483

I'm not sure if we want or care about these messages? Another alternative is to define a new deprecation warning like NodeDeprecationWarning and just turn on those.

Actions #25

Updated by bmbouter about 6 years ago

@daviddavis, I thought we had deprecated nodes in an earlier release, but in searching for the details, I see that we have not declared it deprecated. It looks like the release note checklist item you are doing will be the official nodes deprecation. I've added another checklist item to add a big deprecation note on the nodes.rst page.

Actions #26

Updated by bmbouter about 6 years ago

Let's not enable all deprecation warnings. I really like the idea of declaring a NodeDeprecationWarning and just enabling that one.

Does it make any sense to write a blog post about nodes being deprecated with 2.12 and giving specific recommendations for how users can switch from nodes to a natural sync? I think that would be good. We could link to that blog post from the deprecation statement in the release notes and on nodes.rst pages. That should either become a checklist item or its own task. I think doing it all as 1 task would be good. Whatever you decide, would you be willing to track that work (as checklist item or its own task)?

FYI, here is a draft post on writing blog posts that @dkliban is working on: https://github.com/pulp/pulpproject.org/pull/29

Thank you for doing all of this, it's a big job!

Actions #27

Updated by daviddavis about 6 years ago

Actions #28

Updated by mhrivnak about 6 years ago

  • Sprint/Milestone changed from 29 to 30
Actions #29

Updated by daviddavis about 6 years ago

Actions #30

Updated by daviddavis about 6 years ago

Adding in deprecation warnings to the parent/child code is kind of tricky because there are often multiple places where I can add them. For example, when I bind a repo to a node, we could add a deprecation warning here:

https://git.io/v10R2

But it looks like this also gets called:

https://git.io/v10RM

Also it looks like there are places in the code that I am not sure how to reach. Here's an example:

https://git.io/v10R9

Actions #31

Updated by bmbouter about 6 years ago

I think the fact that [0] is called by [1] is fine because [1] is the publish repo inside of the pulp_node package so I expect [1] is only called for node calls.

Regarding [2], I believe the platform will call that when a orphan delete occurs. The platform would be running an orphan delete celery task through the node workflow. @jortel could confirm this I think.

[0]: https://git.io/v10R2
[1]: https://git.io/v10RM
[2]: https://git.io/v10R9

Actions #32

Updated by daviddavis about 6 years ago

Actions #33

Updated by daviddavis about 6 years ago

Added by daviddavis about 6 years ago

Revision 490203cf

Deprecate Pulp nodes

Update the documentation and release notes to indicate that nodes are deprecated. Also, add warnings in the parent/child tasks and CLI commands.

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

Added by daviddavis about 6 years ago

Revision 490203cf

Deprecate Pulp nodes

Update the documentation and release notes to indicate that nodes are deprecated. Also, add warnings in the parent/child tasks and CLI commands.

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

Actions #34

Updated by daviddavis about 6 years ago

  • Status changed from ASSIGNED to MODIFIED
  • % Done changed from 0 to 100
Actions #35

Updated by semyers about 6 years ago

  • Platform Release set to 2.12.0
Actions #36

Updated by semyers about 6 years ago

  • Status changed from MODIFIED to 5
Actions #37

Updated by semyers almost 6 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE
Actions #38

Updated by bmbouter almost 5 years ago

  • Sprint set to Sprint 12
Actions #39

Updated by bmbouter almost 5 years ago

  • Sprint/Milestone deleted (30)
Actions #40

Updated by bmbouter almost 4 years ago

  • Tags Pulp 2 added

Also available in: Atom PDF