refactor: deprecate accessing constants from top-level namespace

We are planning on adding enumerated constants into gitlab/const.py,
but if we do that than they will end up being added to the top-level
gitlab namespace. We really want to get users to start using
`gitlab.const.` to access the constant values in the future.

Add the currently defined constants to a list that should not change.
Use a module level __getattr__ function so that we can deprecate
access to the top-level constants.

Add a unit test which verifies we generate a warning when accessing
the top-level constants.

3 files changed, 83 insertions, 17 deletions

diff --git a/gitlab/__init__.py b/gitlab/__init__.py
index 7b79f22..824f177 100644
--- a/gitlab/__init__.py
+++ b/gitlab/__init__.py
@@ -17,6 +17,7 @@
 """Wrapper for the GitLab API."""
 
 import warnings
+from typing import Any
 
 import gitlab.config  # noqa: F401
 from gitlab.__version__ import (  # noqa: F401
@@ -28,7 +29,22 @@ from gitlab.__version__ import (  # noqa: F401
     __version__,
 )
 from gitlab.client import Gitlab, GitlabList  # noqa: F401
-from gitlab.const import *  # noqa: F401,F403
 from gitlab.exceptions import *  # noqa: F401,F403
 
 warnings.filterwarnings("default", category=DeprecationWarning, module="^gitlab")
+
+
+# NOTE(jlvillal): We are deprecating access to the gitlab.const values which
+# were previously imported into this namespace by the
+# 'from gitlab.const import *' statement.
+def __getattr__(name: str) -> Any:
+    # Deprecate direct access to constants without namespace
+    if name in gitlab.const._DEPRECATED:
+        warnings.warn(
+            f"\nDirect access to 'gitlab.{name}' is deprecated and will be "
+            f"removed in a future major python-gitlab release. Please "
+            f"use 'gitlab.const.{name}' instead.",
+            DeprecationWarning,
+        )
+        return getattr(gitlab.const, name)
+    raise AttributeError(f"module {__name__} has no attribute {name}")
diff --git a/gitlab/const.py b/gitlab/const.py
index 12faf88..48aa96d 100644
--- a/gitlab/const.py
+++ b/gitlab/const.py
@@ -17,6 +17,41 @@
 
 from gitlab.__version__ import __title__, __version__
 
+# NOTE(jlvillal): '_DEPRECATED' only affects users accessing constants via the
+# top-level gitlab.* namespace. See 'gitlab/__init__.py:__getattr__()' for the
+# consumer of '_DEPRECATED' For example 'x = gitlab.NO_ACCESS'.  We want users
+# to instead use constants by doing code like: gitlab.const.NO_ACCESS.
+_DEPRECATED = [
+    "DEFAULT_URL",
+    "DEVELOPER_ACCESS",
+    "GUEST_ACCESS",
+    "MAINTAINER_ACCESS",
+    "MINIMAL_ACCESS",
+    "NO_ACCESS",
+    "NOTIFICATION_LEVEL_CUSTOM",
+    "NOTIFICATION_LEVEL_DISABLED",
+    "NOTIFICATION_LEVEL_GLOBAL",
+    "NOTIFICATION_LEVEL_MENTION",
+    "NOTIFICATION_LEVEL_PARTICIPATING",
+    "NOTIFICATION_LEVEL_WATCH",
+    "OWNER_ACCESS",
+    "REPORTER_ACCESS",
+    "SEARCH_SCOPE_BLOBS",
+    "SEARCH_SCOPE_COMMITS",
+    "SEARCH_SCOPE_GLOBAL_SNIPPET_TITLES",
+    "SEARCH_SCOPE_ISSUES",
+    "SEARCH_SCOPE_MERGE_REQUESTS",
+    "SEARCH_SCOPE_MILESTONES",
+    "SEARCH_SCOPE_PROJECT_NOTES",
+    "SEARCH_SCOPE_PROJECTS",
+    "SEARCH_SCOPE_USERS",
+    "SEARCH_SCOPE_WIKI_BLOBS",
+    "USER_AGENT",
+    "VISIBILITY_INTERNAL",
+    "VISIBILITY_PRIVATE",
+    "VISIBILITY_PUBLIC",
+]
+
 DEFAULT_URL: str = "https://gitlab.com"
 
 NO_ACCESS: int = 0
diff --git a/tests/unit/test_gitlab.py b/tests/unit/test_gitlab.py
index c147fa0..688da07 100644
--- a/tests/unit/test_gitlab.py
+++ b/tests/unit/test_gitlab.py
@@ -17,12 +17,12 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import pickle
+import warnings
 
 import pytest
 from httmock import HTTMock, response, urlmatch, with_httmock  # noqa
 
-from gitlab import DEFAULT_URL, Gitlab, GitlabList, USER_AGENT
-from gitlab.v4.objects import CurrentUser
+import gitlab
 
 localhost = "http://localhost"
 username = "username"
@@ -94,7 +94,7 @@ def test_gitlab_build_list(gl):
 @with_httmock(resp_page_1, resp_page_2)
 def test_gitlab_all_omitted_when_as_list(gl):
     result = gl.http_list("/tests", as_list=False, all=True)
-    assert isinstance(result, GitlabList)
+    assert isinstance(result, gitlab.GitlabList)
 
 
 def test_gitlab_strip_base_url(gl_trailing):
@@ -114,7 +114,7 @@ def test_gitlab_pickability(gl):
     original_gl_objects = gl._objects
     pickled = pickle.dumps(gl)
     unpickled = pickle.loads(pickled)
-    assert isinstance(unpickled, Gitlab)
+    assert isinstance(unpickled, gitlab.Gitlab)
     assert hasattr(unpickled, "_objects")
     assert unpickled._objects == original_gl_objects
 
@@ -124,24 +124,24 @@ def test_gitlab_token_auth(gl, callback=None):
     gl.auth()
     assert gl.user.username == username
     assert gl.user.id == user_id
-    assert isinstance(gl.user, CurrentUser)
+    assert isinstance(gl.user, gitlab.v4.objects.CurrentUser)
 
 
 def test_gitlab_default_url():
-    gl = Gitlab()
-    assert gl.url == DEFAULT_URL
+    gl = gitlab.Gitlab()
+    assert gl.url == gitlab.DEFAULT_URL
 
 
 @pytest.mark.parametrize(
     "args, kwargs, expected_url, expected_private_token, expected_oauth_token",
     [
-        ([], {}, DEFAULT_URL, None, None),
-        ([None, token], {}, DEFAULT_URL, token, None),
+        ([], {}, gitlab.DEFAULT_URL, None, None),
+        ([None, token], {}, gitlab.DEFAULT_URL, token, None),
         ([localhost], {}, localhost, None, None),
         ([localhost, token], {}, localhost, token, None),
         ([localhost, None, token], {}, localhost, None, token),
-        ([], {"private_token": token}, DEFAULT_URL, token, None),
-        ([], {"oauth_token": token}, DEFAULT_URL, None, token),
+        ([], {"private_token": token}, gitlab.DEFAULT_URL, token, None),
+        ([], {"oauth_token": token}, gitlab.DEFAULT_URL, None, token),
         ([], {"url": localhost}, localhost, None, None),
         ([], {"url": localhost, "private_token": token}, localhost, token, None),
         ([], {"url": localhost, "oauth_token": token}, localhost, None, token),
@@ -162,7 +162,7 @@ def test_gitlab_default_url():
 def test_gitlab_args_kwargs(
     args, kwargs, expected_url, expected_private_token, expected_oauth_token
 ):
-    gl = Gitlab(*args, **kwargs)
+    gl = gitlab.Gitlab(*args, **kwargs)
     assert gl.url == expected_url
     assert gl.private_token == expected_private_token
     assert gl.oauth_token == expected_oauth_token
@@ -170,11 +170,11 @@ def test_gitlab_args_kwargs(
 
 def test_gitlab_from_config(default_config):
     config_path = default_config
-    Gitlab.from_config("one", [config_path])
+    gitlab.Gitlab.from_config("one", [config_path])
 
 
 def test_gitlab_subclass_from_config(default_config):
-    class MyGitlab(Gitlab):
+    class MyGitlab(gitlab.Gitlab):
         pass
 
     config_path = default_config
@@ -185,10 +185,25 @@ def test_gitlab_subclass_from_config(default_config):
 @pytest.mark.parametrize(
     "kwargs,expected_agent",
     [
-        ({}, USER_AGENT),
+        ({}, gitlab.USER_AGENT),
         ({"user_agent": "my-package/1.0.0"}, "my-package/1.0.0"),
     ],
 )
 def test_gitlab_user_agent(kwargs, expected_agent):
-    gl = Gitlab("http://localhost", **kwargs)
+    gl = gitlab.Gitlab("http://localhost", **kwargs)
     assert gl.headers["User-Agent"] == expected_agent
+
+
+def test_gitlab_deprecated_const():
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        gitlab.NO_ACCESS
+    assert len(caught_warnings) == 1
+    warning = caught_warnings[0]
+    assert isinstance(warning.message, DeprecationWarning)
+    message = str(caught_warnings[0].message)
+    assert "deprecated" in message
+    assert "gitlab.const.NO_ACCESS" in message
+
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        gitlab.const.NO_ACCESS
+    assert len(caught_warnings) == 0