-
Notifications
You must be signed in to change notification settings - Fork 120
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
feat: add GetEnvironments method to DBus register #3462
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
# Copyright (c) 2024 Red Hat, Inc. | ||
# | ||
# This software is licensed to you under the GNU General Public License, | ||
# version 2 (GPLv2). There is NO WARRANTY for this software, express or | ||
# implied, including the implied warranties of MERCHANTABILITY or FITNESS | ||
# FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2 | ||
# along with this software; if not, see | ||
# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt. | ||
# | ||
# Red Hat trademarks are not licensed under GPLv2. No permission is | ||
# granted to use or replicate Red Hat trademarks that are incorporated | ||
# in this software or its documentation. | ||
import logging | ||
from typing import List | ||
|
||
|
||
from rhsm.connection import UEPConnection | ||
|
||
log = logging.getLogger(__name__) | ||
|
||
|
||
class EnvironmentService: | ||
""" | ||
Class for using environments | ||
""" | ||
|
||
def __init__(self, cp: UEPConnection) -> None: | ||
""" | ||
Initialization of EnvironmentService instance | ||
:param cp: connection to Candlepin | ||
""" | ||
self.cp = cp | ||
|
||
def list(self, org_id: str) -> List[dict]: | ||
""" | ||
Method for listing environments | ||
:param org_id: organization to list environments for | ||
:return: List of environments. | ||
""" | ||
list_all = self.cp.has_capability("typed_environments") | ||
environments = self.cp.getEnvironmentList(org_id, list_all=list_all) | ||
|
||
return environments |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,77 @@ | |
] | ||
""" | ||
|
||
ENVIRONMENTS_CONTENT_JSON = """[ | ||
{ | ||
"created": "2024-11-07T20:01:47+0000", | ||
"updated": "2024-11-07T20:01:47+0000", | ||
"id": "fake-id", | ||
"name": "test-environment", | ||
"description": "test description", | ||
"contentPrefix": null, | ||
"type": "content-template", | ||
"environmentContent": [] | ||
}, | ||
{ | ||
"created": "2024-11-07T20:01:47+0000", | ||
"updated": "2024-11-07T20:01:47+0000", | ||
"id": "fake-id-2", | ||
"name": "test-environment-2", | ||
"description": "test description", | ||
"contentPrefix": null, | ||
"type": "content-template", | ||
"environmentContent": [] | ||
}, | ||
{ | ||
"created": "2024-11-07T20:01:47+0000", | ||
"updated": "2024-11-07T20:01:47+0000", | ||
"id": "fake-id-3", | ||
"name": "test-environment-3", | ||
"description": "test description", | ||
"contentPrefix": null, | ||
"type": null, | ||
"environmentContent": [] | ||
}, | ||
{ | ||
"created": "2024-11-07T20:01:47+0000", | ||
"updated": "2024-11-07T20:01:47+0000", | ||
"id": "fake-id-4", | ||
"name": "test-environment-4", | ||
"description": "test description", | ||
"contentPrefix": null, | ||
"environmentContent": [] | ||
} | ||
] | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add more environments to this list. You will see that new unit test will start to fail. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added more environments to this list (10 more), but I don't see an issue. If you're seeing something else, what else can I do to reproduce? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the old code was used, then the unit test would fail. |
||
|
||
ENVIRONMENTS_DBUS_JSON = """[ | ||
{ | ||
"id": "fake-id", | ||
"name": "test-environment", | ||
"description": "test description", | ||
"type": "content-template" | ||
}, | ||
{ | ||
"id": "fake-id-2", | ||
"name": "test-environment-2", | ||
"description": "test description", | ||
"type": "content-template" | ||
}, | ||
{ | ||
"id": "fake-id-3", | ||
"name": "test-environment-3", | ||
"description": "test description", | ||
"type": "" | ||
}, | ||
{ | ||
"id": "fake-id-4", | ||
"name": "test-environment-4", | ||
"description": "test description", | ||
"type": "" | ||
} | ||
] | ||
""" | ||
|
||
|
||
class RegisterDBusObjectTest(SubManDBusFixture): | ||
socket_dir: Optional[tempfile.TemporaryDirectory] = None | ||
|
@@ -308,6 +379,21 @@ def test_GetOrgs(self): | |
result = self.impl.get_organizations({"username": "username", "password": "password"}) | ||
self.assertEqual(expected, result) | ||
|
||
def test_GetEnvironments(self): | ||
self.patches["is_registered"].return_value = False | ||
mock_cp = mock.Mock(spec=connection.UEPConnection, name="UEPConnection") | ||
mock_cp.username = "username" | ||
mock_cp.password = "password" | ||
mock_cp.getEnvironmentList = mock.Mock() | ||
mock_cp.getEnvironmentList.return_value = json.loads(ENVIRONMENTS_CONTENT_JSON) | ||
self.patches["build_uep"].return_value = mock_cp | ||
|
||
expected = json.loads(ENVIRONMENTS_DBUS_JSON) | ||
result = self.impl.get_environments( | ||
{"username": "username", "password": "password", "org_id": "org_id"} | ||
) | ||
self.assertEqual(expected, result) | ||
|
||
def test_RegisterWithActivationKeys(self): | ||
expected = json.loads(CONSUMER_CONTENT_JSON_SCA) | ||
self.patches["is_registered"].return_value = False | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
# Copyright (c) 2024 Red Hat, Inc. | ||
# | ||
# This software is licensed to you under the GNU General Public License, | ||
# version 2 (GPLv2). There is NO WARRANTY for this software, express or | ||
# implied, including the implied warranties of MERCHANTABILITY or FITNESS | ||
# FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2 | ||
# along with this software; if not, see | ||
# http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt. | ||
# | ||
# Red Hat trademarks are not licensed under GPLv2. No permission is | ||
# granted to use or replicate Red Hat trademarks that are incorporated | ||
# in this software or its documentation. | ||
from unittest import mock | ||
|
||
from test.rhsmlib.base import InjectionMockingTest | ||
|
||
from rhsm import connection | ||
|
||
from rhsmlib.services import environment | ||
|
||
|
||
ENVIRONMENTS_JSON = [ | ||
{ | ||
"created": "2024-10-03T18:12:56+0000", | ||
"updated": "2024-10-03T18:12:56+0000", | ||
"id": "8bdf14cf9e534119a1fe617c03304768", | ||
"name": "template 1", | ||
"type": "content-template", | ||
"description": "my template", | ||
"owner": { | ||
"id": "8ad980939253781c01925378340e0002", | ||
"key": "content-sources-test", | ||
"displayName": "ContentSourcesTest", | ||
"href": "/owners/content-sources-test", | ||
"contentAccessMode": "org_environment", | ||
}, | ||
"environmentContent": [ | ||
{"contentId": "11055", "enabled": True}, | ||
{"contentId": "56a3a98c76ea4e16bd68424a2c9cc1c1", "enabled": True}, | ||
{"contentId": "11049", "enabled": True}, | ||
], | ||
}, | ||
{ | ||
"created": "2024-10-09T19:08:14+0000", | ||
"updated": "2024-10-09T19:08:14+0000", | ||
"id": "6c62889601be41128fe2fece53141fd4", | ||
"name": "template 2", | ||
"type": "content-template", | ||
"description": "my template", | ||
"owner": { | ||
"id": "8ad980939253781c01925378340e0002", | ||
"key": "content-sources-test", | ||
"displayName": "ContentSourcesTest", | ||
"href": "/owners/content-sources-test", | ||
"contentAccessMode": "org_environment", | ||
}, | ||
"environmentContent": [ | ||
{"contentId": "11055", "enabled": True}, | ||
{"contentId": "11049", "enabled": True}, | ||
], | ||
}, | ||
] | ||
|
||
|
||
class TestEnvironmentService(InjectionMockingTest): | ||
def setUp(self): | ||
super(TestEnvironmentService, self).setUp() | ||
self.mock_cp = mock.Mock(spec=connection.UEPConnection, name="UEPConnection") | ||
|
||
def injection_definitions(self, *args, **kwargs): | ||
return None | ||
|
||
def test_list_environments(self): | ||
self.mock_cp.getEnvironmentList.return_value = ENVIRONMENTS_JSON | ||
|
||
result = environment.EnvironmentService(self.mock_cp).list("org") | ||
self.assertEqual(ENVIRONMENTS_JSON, result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method should not be in
rhsmlib.dbus.objects.register
module. This module should contain only methods related to D-Bus. This proposed method should be in newrhsmlib.services.environment
module, because you can see that subscription-manager CLI already hasenvironments
sub-command. Thus, there will be potential to share new code.This method should have optional argument
uep
(basic auth connection to candlepin) to be able to pass existing connection instance to this method.I this method had another argument
typed_environments: bool = True
, then we could list not only typed environments. Iftyped_environments is False
, then we could skip detection for that capability on server.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding of the card is that the GetEnvironments() method is intended to be a DBus method. With that said, I moved the actual list call to candlepin to a newly created EnvironmentService class. I then instantiated that in the DBus method and used it to call to candlepin. Is that okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm misunderstanding your request, I'm happy to make more changes :)