Project

Profile

Help

Task #1488

Deprecate nodes

Added by bmbouter about 4 years ago. Updated 11 months ago.

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

100%

Platform Release:
2.12.0
Blocks Release:
Backwards Incompatible:
No
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
Pulp 2
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 12

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


Checklist


Related issues

Blocks Pulp - Task #2460: Write a blog post about nodes being deprecated and what functionality users can use to replace them CLOSED - CURRENTRELEASE Actions

Associated revisions

Revision 490203cf View on GitHub
Added by daviddavis about 3 years ago

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

Revision 490203cf View on GitHub
Added by daviddavis about 3 years ago

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

History

#1 Updated by mhrivnak about 4 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

#2 Updated by mhrivnak about 4 years ago

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

#3 Updated by mhrivnak almost 4 years ago

  • Sprint Candidate changed from Yes to No

#4 Updated by amacdona@redhat.com over 3 years ago

  • Sprint Candidate changed from No to Yes

#5 Updated by mhrivnak over 3 years ago

  • Sprint/Milestone set to 27

#6 Updated by jortel@redhat.com over 3 years ago

  • Sprint/Milestone changed from 27 to 28

#7 Updated by daviddavis over 3 years ago

  • Assignee set to daviddavis

#8 Updated by ipanova@redhat.com over 3 years ago

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

#9 Updated by daviddavis over 3 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to daviddavis

#10 Updated by daviddavis over 3 years ago

  • Checklist item for a node_child, in a core nodes task performed on a pulp_worker, loudly log a DeprecationWarning set to Done

#11 Updated by daviddavis over 3 years ago

  • Checklist item for a node_child, in a core nodes task performed on a pulp_worker, loudly log a DeprecationWarning set to Not done

#12 Updated by daviddavis over 3 years ago

  • Checklist item log in the foreground a deprecation notice for any pulp node consumer command added

#13 Updated by daviddavis over 3 years ago

  • Checklist item add a release note indicating the deprecation and suggesting users do a natural sync instead set to Done

#14 Updated by daviddavis over 3 years ago

  • Checklist item log in the foreground a banner output indicating deprecation for any pulp-admin command starting with `pulp-admin node` set to Done
  • Checklist item Add a deprecation banner to the top of the nodes docs suggesting that users do a natural sync instead set to Done
  • Checklist item log in the foreground a deprecation notice for any pulp node consumer command set to Done

#15 Updated by daviddavis about 3 years ago

  • Checklist item for a node_parent, in a core nodes task performed on a pulp_worker, loudly log DeprecationWarning set to Done

#16 Updated by daviddavis about 3 years ago

  • Checklist item for a node_parent, in a core nodes task performed on a pulp_worker, loudly log DeprecationWarning set to Not done

#17 Updated by daviddavis about 3 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

#18 Updated by daviddavis about 3 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

#19 Updated by bmbouter about 3 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.

#20 Updated by daviddavis about 3 years ago

  • Checklist item Create a blog post showing how to sync content between pulp instances added
  • Checklist item Email the pulp mailing list to announce nodes are being removed added

#21 Updated by daviddavis about 3 years ago

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

#22 Updated by daviddavis about 3 years ago

  • Checklist item deleted (Create a blog post showing how to sync content between pulp instances)
  • Checklist item deleted (Email the pulp mailing list to announce nodes are being removed)

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

#23 Updated by daviddavis about 3 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?

#24 Updated by daviddavis about 3 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.

#25 Updated by bmbouter about 3 years ago

  • Checklist item add a big deprecation note in the nodes documentation page (docs/user-guide/nodes.rst) added

@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.

#26 Updated by bmbouter about 3 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!

#27 Updated by daviddavis about 3 years ago

  • Checklist item add a big deprecation note in the nodes documentation page (docs/user-guide/nodes.rst) set to Done
  • Checklist item add a release note indicating the deprecation and suggesting users do a natural sync instead set to Not done

#28 Updated by mhrivnak about 3 years ago

  • Sprint/Milestone changed from 29 to 30

#29 Updated by daviddavis about 3 years ago

  • Checklist item add a release note indicating the deprecation and suggesting users do a natural sync instead set to Done

#30 Updated by daviddavis about 3 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

#31 Updated by bmbouter about 3 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

#32 Updated by daviddavis about 3 years ago

  • Checklist item for a node_child, in a core nodes task performed on a pulp_worker, loudly log a DeprecationWarning set to Done

#33 Updated by daviddavis about 3 years ago

  • Checklist item for a node_parent, in a core nodes task performed on a pulp_worker, loudly log DeprecationWarning set to Done

#34 Updated by daviddavis about 3 years ago

  • Status changed from ASSIGNED to MODIFIED
  • % Done changed from 0 to 100

#35 Updated by semyers about 3 years ago

  • Platform Release set to 2.12.0

#36 Updated by semyers about 3 years ago

  • Status changed from MODIFIED to ON_QA

#37 Updated by semyers about 3 years ago

  • Status changed from ON_QA to CLOSED - CURRENTRELEASE

#38 Updated by bmbouter almost 2 years ago

  • Sprint set to Sprint 12

#39 Updated by bmbouter almost 2 years ago

  • Sprint/Milestone deleted (30)

#40 Updated by bmbouter 11 months ago

  • Tags Pulp 2 added

Please register to edit this issue

Also available in: Atom PDF