Skip to content

Commit

Permalink
Drop PDC support entirely
Browse files Browse the repository at this point in the history 
The 'grouped critical path' approach using JSON files stored
on the Bodhi server directly seems to be working out great in
production after a few months. So let's just drop the PDC support
entirely, it's no longer needed and it makes the code simpler.

Signed-off-by: Adam Williamson <[email protected]>
  • Loading branch information
@AdamWill
AdamWill committed Jul 20, 2023
1 parent 0f73f1d commit 94cb9f6
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 430 deletions.
3 changes: 0 additions & 3 deletions bodhi-server/bodhi/server/config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,6 @@ class BodhiConfig(dict):
'pagure_url': {
'value': 'https://src.fedoraproject.org/pagure/',
'validator': _validate_tls_url},
'pdc_url': {
'value': 'https://pdc.fedoraproject.org/',
'validator': _validate_tls_url},
'privacy_link': {
'value': '',
'validator': str},
Expand Down
5 changes: 0 additions & 5 deletions bodhi-server/bodhi/server/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2194,8 +2194,6 @@ def contains_critpath_component(builds, release_branch):
such as "f25" or "master".
Returns:
bool: ``True`` if the update contains a critical path package, ``False`` otherwise.
Raises:
RuntimeError: If the PDC did not give us a 200 code.
"""
components = build_names_by_type(builds)

Expand Down Expand Up @@ -2441,8 +2439,6 @@ def new(cls, request, data):
data (dict): A key-value mapping of the new update's attributes.
Returns:
tuple: A 2-tuple of the edited update and a list of dictionaries that describe caveats.
Raises:
RuntimeError: If the PDC did not give us a 200 code.
"""
db = request.db
user = User.get(request.user.name)
Expand Down Expand Up @@ -2556,7 +2552,6 @@ def edit(cls, request, data):
tuple: A 2-tuple of the edited update and a list of dictionaries that describe caveats.
Raises:
LockedUpdateException: If the update is locked.
RuntimeError: If the PDC did not give us a 200 code.
"""
db = request.db
buildinfo = request.buildinfo
Expand Down
102 changes: 11 additions & 91 deletions bodhi-server/bodhi/server/util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
"""Random functions that don't fit elsewhere."""

from collections import defaultdict, OrderedDict
from collections import defaultdict
from contextlib import contextmanager
from datetime import datetime, timedelta
from importlib import import_module
Expand Down Expand Up @@ -197,8 +197,7 @@ def get_grouped_critpath_components(collection='master', component_type='rpm', c
Args:
collection (str): The collection/branch to search. Defaults to 'master'.
component_type (str): The component type to search for. This only affects PDC
queries. Defaults to 'rpm'.
component_type (str): The component type to search for. Defaults to 'rpm'.
components (frozenset or None): The list of components we are interested in. If None (the
default), all components for the given collection and type are returned.
Returns:
Expand All @@ -208,6 +207,8 @@ def get_grouped_critpath_components(collection='master', component_type='rpm', c
"""
critpath_type = config.get('critpath.type')
if critpath_type != 'json':
if not critpath_type:
critpath_type = "(default)"
raise ValueError(f'critpath.type {critpath_type} does not support groups')
critpath_components = {}
try:
Expand All @@ -231,26 +232,22 @@ def get_critpath_components(collection='master', component_type='rpm', component
Return a list of critical path packages for a given collection, filtered by components.
Args:
collection (str): The collection/branch to search. Defaults to 'master'.
component_type (str): The component type to search for. This only affects PDC
queries. Defaults to 'rpm'.
collection (str): The collection/branch to search. Defaults to 'master'. Only
has any effect when critpath_type is json.
component_type (str): The component type to search for. Defaults to 'rpm'. Only
has any effect when critpath_type is json.
components (frozenset or None): The list of components we are interested in. If None (the
default), all components for the given collection and type are returned.
Returns:
list: The critpath components for the given collection and type.
Raises:
RuntimeError: If the PDC did not give us a 200 code.
"""
critpath_components = []
critpath_type = config.get('critpath.type')
if critpath_type not in ('pdc', 'json') and component_type != 'rpm':
if critpath_type != 'json' and component_type != 'rpm':
log.warning('The critpath.type of "{0}" does not support searching for'
' non-RPM components'.format(component_type))
' non-RPM components'.format(critpath_type or "(default)"))

if critpath_type == 'pdc':
critpath_components = get_critpath_components_from_pdc(
collection, component_type, components)
elif critpath_type == 'json':
if critpath_type == 'json':
try:
critpath_components_grouped = read_critpath_json(collection).get(component_type, {})
for compgroup in critpath_components_grouped.values():
Expand Down Expand Up @@ -979,67 +976,6 @@ def severity_updateinfo_str(value):
return severity_map.get(value, "None")


# If we need to know about more components than this constant, we will just get the full
# list, rather than a query per package. This is because at some point, just going through
# paging becomes more performant than getting the page for every component.
PDC_CRITPATH_COMPONENTS_GETALL_LIMIT = 10


@memoized
def get_critpath_components_from_pdc(branch, component_type='rpm', components=None):
"""
Search PDC for critical path packages based on the specified branch.
Args:
branch (str): The branch name to search by.
component_type (str): The component type to search by. Defaults to ``rpm``.
components (frozenset or None): The list of components we are interested in. If None (the
default), all components for the given branch and type are returned.
Returns:
list: Critical path package names.
Raises:
RuntimeError: If the PDC did not give us a 200 code.
"""
pdc_api_url = '{}/rest_api/v1/component-branches/'.format(
config.get('pdc_url').rstrip('/'))
query_args = {
'active': 'true',
'critical_path': 'true',
'name': branch,
'page_size': 100,
'type': component_type,
'fields': 'global_component'
}
# Create ordered dictionary with sorted query args to be able to compare URLs
query_args = OrderedDict(sorted(query_args.items(), key=lambda x: x[0]))

critpath_pkgs_set = set()
if components and len(components) < PDC_CRITPATH_COMPONENTS_GETALL_LIMIT:
# Do a query for every single component
for component in components:
query_args['global_component'] = component
pdc_api_url_with_args = '{0}?{1}'.format(pdc_api_url, urlencode(query_args))
pdc_request_json = pdc_api_get(pdc_api_url_with_args)
for branch_rv in pdc_request_json['results']:
critpath_pkgs_set.add(branch_rv['global_component'])
if pdc_request_json['next']:
raise Exception('We got paging when requesting a single component?!')
else:
pdc_api_url_with_args = '{0}?{1}'.format(pdc_api_url, urlencode(query_args))
while True:
pdc_request_json = pdc_api_get(pdc_api_url_with_args)

for branch_rv in pdc_request_json['results']:
critpath_pkgs_set.add(branch_rv['global_component'])

if pdc_request_json['next']:
pdc_api_url_with_args = pdc_request_json['next']
else:
# There are no more results to iterate through
break
return list(critpath_pkgs_set)


def read_critpath_json(collection):
"""
Read the JSON format critical path information for the collection.
Expand Down Expand Up @@ -1141,22 +1077,6 @@ def pagure_api_get(pagure_api_url):
return call_api(pagure_api_url, service_name='Pagure', error_key='error', retries=3)


def pdc_api_get(pdc_api_url):
"""
Perform a GET request against PDC.
Args:
pdc_api_url (str): The URL to GET, including query parameters.
Returns:
dict: A dictionary response representing the API response's JSON.
Raises:
RuntimeError: If the server did not give us a 200 code.
"""
# There is no error_key specified because the error key is not consistent
# based on the error message
return call_api(pdc_api_url, service_name='PDC', retries=3)


def greenwave_api_post(greenwave_api_url, data):
"""
Post a request to Greenwave.
Expand Down
18 changes: 6 additions & 12 deletions bodhi-server/production.ini
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,6 @@ use = egg:bodhi-server
# Values are in the form `PackageType:PagureNamespace`
# pagure_namespaces = rpm:rpms, module:modules, container:container, flatpak:flatpaks

##
## Product Definition Center (PDC)
##
# pdc_url = https://pdc.fedoraproject.org/


##
## Bug tracker settings
Expand Down Expand Up @@ -395,20 +390,19 @@ use = egg:bodhi-server
## https://fedoraproject.org/wiki/Critical_path_package
##

# You can allow Bodhi to query for critpath packages from the Product Definition
# Center by setting this value to `pdc`, or to read critpath packages from JSON
# files (expected to be generated by the `critpath.py` script from the releng
# repo) by setting this value to `json`. If it isn't set, it'll just use the
# hardcoded list below.
# You can allow Bodhi to read critpath packages from JSON files (expected to
# be generated by the `critpath.py` script from the releng repo) by setting
# this value to `json`. If it isn't set, it'll just use the hardcoded list
# below.
# critpath.type =

# If critpath.type is set to `json`, this value sets the directory where Bodhi
# will look for the JSON files. The filenames should match Bodhi release
# 'branch' values, which should be the name of the git branch for the release.
# critpath.jsonpath = /etc/bodhi/critpath

# You can hardcode a list of critical path packages instead of using the PkgDB
# or PDC. This is used if critpath.type is not defined.
# You can hardcode a list of critical path packages instead of using JSON
# files. This is used if critpath.type is not defined.
# critpath_pkgs =

# The number of admin approvals it takes to be able to push a critical path
Expand Down
66 changes: 6 additions & 60 deletions bodhi-server/tests/test_models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3281,79 +3281,25 @@ def test_modify_bugs_stable_no_close(self, comment, close):
# No bugs should have been closed
assert close.call_count == 0

@mock.patch('bodhi.server.util.http_session')
@mock.patch.dict(util.config, {
'critpath.type': 'pdc',
'pdc_url': 'http://domain.local'
'critpath.type': None,
'critpath_pkgs': 'gcc TurboGears',
})
def test_contains_critpath_component(self, session):
def test_contains_critpath_component(self):
""" Verifies that the static function of contains_critpath_component
determines that one of the builds has a critpath component.
"""
session.get.return_value.status_code = 200
session.get.return_value.json.return_value = {
'count': 2,
'next': None,
'previous': None,
'results': [
{
'active': True,
'critical_path': True,
'global_component': 'gcc',
'id': 6,
'name': 'f11',
'slas': [],
'type': 'rpm'
},
{
'active': True,
'critical_path': True,
'global_component': 'TurboGears',
'id': 7,
'name': 'f11',
'slas': [],
'type': 'rpm'
}
]
}
update = self.get_update()
assert update.contains_critpath_component(update.builds, update.release.name)

@mock.patch('bodhi.server.util.http_session')
@mock.patch.dict(util.config, {
'critpath.type': 'pdc',
'pdc_url': 'http://domain.local'
'critpath.type': None,
'critpath_pkgs': 'gcc python',
})
def test_contains_critpath_component_not_critpath(self, session):
def test_contains_critpath_component_not_critpath(self):
""" Verifies that the static function of contains_critpath_component
determines that none of the builds are critpath components.
"""
session.get.return_value.status_code = 200
session.get.return_value.json.return_value = {
'count': 2,
'next': None,
'previous': None,
'results': [
{
'active': True,
'critical_path': True,
'global_component': 'gcc',
'id': 6,
'name': 'f25',
'slas': [],
'type': 'rpm'
},
{
'active': True,
'critical_path': True,
'global_component': 'python',
'id': 7,
'name': 'f25',
'slas': [],
'type': 'rpm'
}
]
}
update = self.get_update()
# Use a different release here for additional testing and to avoid
# caching from the previous test
Expand Down
Loading

0 comments on commit 94cb9f6

Please sign in to comment.