optimizely / python-sdk

Python SDK for Optimizely Feature Experimentation and Optimizely Full Stack (legacy)
https://docs.developers.optimizely.com/experimentation/v4.0.0-full-stack/docs/python-sdk
Apache License 2.0
32 stars 36 forks source link

Removing deprecated warnings #335

Closed The-inside-man closed 3 years ago

The-inside-man commented 3 years ago

Summary

    - assertRaisesRegexp -> assertRaisesRegex
    - assertEquals -> assertEqual
    - isAlive() -> is_alive()
    - Check added to base.py to confirm attribute assertRaisesRegex for backwards compatibility to Python2.7

Test plan

Compatibility Test

keppel2 commented 3 years ago

Have you looked into preventing future warnings (lint)?

The-inside-man commented 3 years ago

Have you looked into preventing future warnings (lint)?

I was looking at the linter, but I wouldn't want to hide valid warnings. Since these deprecations are based on python and supporting libraries being upgraded the warnings are good in the sense it tells us developers to update our code base, but they shouldn't go untouched for too long. So keeping that in mind I don't want to block future warnings, just address them sooner maybe as they come so we are keeping our code base up to date as frequently as possible. For the most part updating to resolve these warnings are very little work and don't take much time. We just need to be concerned with backwards compatibility.

keppel2 commented 3 years ago

Oh, I meant enforce a no-warning policy somehow. It would have to be situational, since some warnings are unavoidable.

The-inside-man commented 3 years ago

Oh, I meant enforce a no-warning policy somehow. It would have to be situational, since some warnings are unavoidable.

ahh, ok , I see what you are saying! we can pass the --disable-warnings flag when using pytest or similar when building which would help for those who want to avoid. In the build scripts, this could be added to clean the output moving forward, we would then not see these warnings on the builds in CI. locally we would still see them unless we add that flag and same for anyone else using the SDK.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 95.871% when pulling 5c64dea6205ff05479e4dcb0ba23f8bd0c5a46d0 on jbrown/testcases-remove-warnings into cdc652ef523b227e2ac3b7880ea6c6634748d0e8 on master.