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

Properly deprecate template context data in Fluorine #35777

Closed
rallytime opened this issue Aug 25, 2016 · 2 comments
Closed

Properly deprecate template context data in Fluorine #35777

rallytime opened this issue Aug 25, 2016 · 2 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix P2 Priority 2 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around ZRELEASED - Fluorine reitred label
Milestone

Comments

@rallytime
Copy link
Contributor

Description of Issue/Question

There were deprecation warnings in salt/utils/__init__.py to change the warnings around passing extra data as key:value pairs to raising a SaltInvocationError. When this removal was made in #35712, many test failures happened on develop in important areas (such as rest_cherrypi and wheel client tests).

In order to avoid bad regressions on the Carbon feature release, the removal was reverted in #35770 and the deprecation release version was bumped back one feature release cycle to Nitrogen in #35776.

The code needs to be audited more carefully to avoid test failures and regressions in other areas that tests might not be catching. Also see this comment and this comment for more information, as there are several fields that need to be added to the CLIENT_INTERNAL_KEYWORDS frozen set in salt/client/mixins.py.

ping @s0undt3ch

@rallytime rallytime added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Core relates to code central or existential to Salt P2 Priority 2 ZRELEASED - 2017.7.0rc1 labels Aug 25, 2016
@rallytime rallytime added this to the Approved milestone Aug 25, 2016
@rallytime
Copy link
Contributor Author

Oh, also the list of tests that failed when the change was merged are the following:

unit.utils.utils_test.UtilsTestCase.test_format_call
integration.netapi.rest_cherrypy.app_test.TestArgKwarg.test_accepts_arg_kwarg_keys
integration.netapi.rest_cherrypy.app_test.TestLogin.test_good_login
integration.netapi.rest_cherrypy.app_test.TestLogin.test_logout
integration.wheel.client.WheelModuleTest.test_cmd_sync
integration.wheel.client.WheelModuleTest.test_cmd_sync_w_arg
integration.wheel.client.WheelModuleTest.test_master_call

I made a fix for the unit.utils.utils_test.UtilsTestCase.test_format_call failure in #35739, but I closed the PR since the fix is not longer needed as the change was reverted. That test fix will need to be made again once this deprecation removal has been officially made.

@meggiebot meggiebot modified the milestones: Nitrogen 2, Approved Dec 19, 2016
@meggiebot meggiebot modified the milestones: Nitrogen 3, Nitrogen 2 Jan 17, 2017
@meggiebot meggiebot assigned rallytime and unassigned cachedout Jan 23, 2017
@rallytime rallytime removed their assignment Jan 25, 2017
@rallytime rallytime modified the milestones: Approved, Nitrogen 3 Jan 25, 2017
rallytime pushed a commit to rallytime/salt that referenced this issue Jan 25, 2017
@rallytime rallytime changed the title Properly deprecate template context data in Nitrogen Properly deprecate template context data in Oxygen Jan 25, 2017
@Ch3LL Ch3LL modified the milestones: Approved, oxygen Nov 22, 2017
@rallytime rallytime changed the title Properly deprecate template context data in Oxygen Properly deprecate template context data in Fluorine Nov 29, 2017
@garethgreenaway
Copy link
Contributor

Updated tests:

unit.utils.test_args.ArgsTestCase.test_format_call
integration.netapi.rest_cherrypy.test_app.TestArgKwarg.test_accepts_arg_kwarg_keys
integration.netapi.rest_cherrypy.test_app.TestLogin.test_good_login
integration.netapi.rest_cherrypy.test_app.TestLogin.test_logout
integration.wheel.test_client.WheelModuleTest.test_cmd_sync
integration.wheel.test_client.WheelModuleTest.test_cmd_sync_w_arg
integration.wheel.test_client.WheelModuleTest.test_master_call

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix P2 Priority 2 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around ZRELEASED - Fluorine reitred label
Projects
None yet
Development

No branches or pull requests

5 participants