Merge pull request galaxyproject#17340 from nsoranzo/flake8-bugbear_2…

…4.1.17

Fixes for flake8-bugbear 24.1.17

.ci/flake8_ignorelist.txt

@@ -1,7 +1,6 @@
 .git
 .tox
-.venv
-.venv3
+.venv*
 packages/*/.venv
 packages/*/build
 packages/*/dist

.gitignore

@@ -7,8 +7,7 @@ scripts/scramble/lib
 scripts/scramble/archives
 
 # Python virtualenv
-.venv
-.venv3
+.venv*/
 
 # Python build artifacts
 build

lib/galaxy/managers/hdas.py

@@ -419,7 +419,7 @@ def cleanup_items(self, user: model.User, item_ids: Set[int]) -> StorageItemsCle
                 dataset_ids_to_remove.add(hda.dataset.id)
                 success_item_count += 1
                 total_free_bytes += quota_amount
-            except BaseException as e:
+            except Exception as e:
                 errors.append(StorageItemCleanupError(item_id=hda_id, error=str(e)))
 
         if success_item_count:

lib/galaxy/managers/histories.py

@@ -626,7 +626,7 @@ def cleanup_items(self, user: model.User, item_ids: Set[int]) -> StorageItemsCle
                 self.history_manager.purge(history, flush=False, user=user)
                 success_item_count += 1
                 total_free_bytes += int(history.disk_size)
-            except BaseException as e:
+            except Exception as e:
                 errors.append(StorageItemCleanupError(item_id=history_id, error=str(e)))
 
         if success_item_count:

lib/galaxy/tool_shed/tools/data_table_manager.py

@@ -163,16 +163,17 @@ def install_tool_data_tables(self, tool_shed_repository, tool_index_sample_files
         if os.path.exists(tool_data_table_conf_filename):
             tree, error_message = xml_util.parse_xml(tool_data_table_conf_filename)
             if tree:
-                for elem in tree.getroot():
-                    # Append individual table elems or other elemes, but not tables elems.
-                    if elem.tag == "tables":
-                        # TODO: this code need to be revised
-                        for _table_elem in elems:
-                            elems.append(elem)
-                    else:
-                        elems.append(elem)
+                root = tree.getroot()
+                if root.tag == "tables":
+                    elems = list(root)
+                else:
+                    log.warning(
+                        "The '%s' data table file has '%s' instead of <tables> as root element, skipping.",
+                        tool_data_table_conf_filename,
+                        root.tag,
+                    )
         else:
-            log.debug(
+            log.warning(
                 "The '%s' data table file was not found, but was expected to be copied from '%s' during repository installation.",
                 tool_data_table_conf_filename,
                 TOOL_DATA_TABLE_FILE_SAMPLE_NAME,

lib/galaxy/tool_shed/util/utility_container_manager.py

@@ -58,9 +58,11 @@ def contains_repository_dependency(self, repository_dependency):
 
     def remove_repository_dependency(self, repository_dependency):
         listified_repository_dependency = repository_dependency.listify
-        for contained_repository_dependency in self.repository_dependencies:
-            if contained_repository_dependency.listify == listified_repository_dependency:
-                self.repository_dependencies.remove(contained_repository_dependency)
+        self.repository_dependencies = [
+            contained_repository_dependency
+            for contained_repository_dependency in self.repository_dependencies
+            if contained_repository_dependency.listify != listified_repository_dependency
+        ]
 
     def to_dict(self):
         folders = []

lib/galaxy/tool_util/data/__init__.py

@@ -1101,10 +1101,7 @@ def to_xml_file(
         except Exception as e:
             out_elems = []
             log.debug("Could not parse existing tool data table config, assume no existing elements: %s", e)
-        for elem in remove_elems:
-            # handle multiple occurrences of remove elem in existing elems
-            while elem in out_elems:
-                remove_elems.remove(elem)
+        out_elems = [elem for elem in out_elems if elem not in remove_elems]
         # add new elems
         out_elems.extend(new_elems)
         out_path_is_new = not os.path.exists(full_path)

lib/galaxy/tool_util/toolbox/base.py

@@ -261,14 +261,14 @@ def _init_tools_from_configs(self, config_filenames):
         execution_timer = ExecutionTimer()
         self._tool_tag_manager.reset_tags()
         config_filenames = listify(config_filenames)
-        for config_filename in config_filenames:
-            if os.path.isdir(config_filename):
-                directory_contents = sorted(os.listdir(config_filename))
-                directory_config_files = [
-                    config_file for config_file in directory_contents if config_file.endswith(".xml")
-                ]
-                config_filenames.remove(config_filename)
-                config_filenames.extend(directory_config_files)
+        config_directories = [config_filename for config_filename in config_filenames if os.path.isdir(config_filename)]
+        config_filenames = [
+            config_filename for config_filename in config_filenames if config_filename not in config_directories
+        ]
+        for config_directory in config_directories:
+            directory_contents = sorted(os.listdir(config_directory))
+            directory_config_files = [config_file for config_file in directory_contents if config_file.endswith(".xml")]
+            config_filenames.extend(directory_config_files)
         for config_filename in config_filenames:
             if not self.can_load_config_file(config_filename):
                 continue

lib/galaxy/tool_util/toolbox/watcher.py

@@ -136,7 +136,7 @@ def check(self):
                     # thread to die in these cases.
                     if path in drop_now:
                         log.warning("'%s' could not be read, removing from watched files", path)
-                        del paths[path]
+                        del self.paths[path]
                         if path in hashes:
                             del hashes[path]
                     else:

lib/galaxy/util/submodules.py

@@ -45,7 +45,7 @@ def __import_submodules_impl(module, recursive=False):
             submodules.append(submodule)
             if recursive and is_pkg:
                 submodules.update(__import_submodules_impl(submodule, recursive=True))
-        except BaseException:
+        except Exception:
             message = f"{full_name} dynamic module could not be loaded (traceback follows):"
             log.exception(message)
             continue

lib/galaxy/util/topsort.py

@@ -168,7 +168,7 @@ def topsort(pairlist):
             for y in successors[x]:
                 numpreds[y] = numpreds[y] - 1
                 if numpreds[y] == 0:
-                    answer.append(y)
+                    answer.append(y)  # noqa: B038
             # following "del" isn't needed; just makes
             # CycleError details easier to grasp
             del successors[x]

lib/galaxy/util/xml_macros.py

@@ -283,13 +283,6 @@ def _load_macro_file(path: StrPath, xml_base_dir, macros) -> List[str]:
     return _load_macros(root, xml_base_dir, macros)
 
 
-def _xml_set_children(element, new_children):
-    for old_child in element:
-        element.remove(old_child)
-    for i, new_child in enumerate(new_children):
-        element.insert(i, new_child)
-
-
 def _xml_replace(query, targets):
     parent_el = query.find("..")
     matching_index = -1

lib/galaxy/webapps/galaxy/services/history_contents.py

@@ -1348,7 +1348,7 @@ def _apply_operation_to_item(
         try:
             self.item_operator.apply(operation, item, params, trans)
             return None
-        except BaseException as exc:
+        except Exception as exc:
             return BulkOperationItemError(
                 item=EncodedHistoryContentItem(id=item.id, history_content_type=item.history_content_type),
                 error=str(exc),

test/unit/app/test_galaxy_install.py

@@ -20,8 +20,8 @@
 
 def test_against_production_shed(tmp_path: Path):
     repo_owner = "iuc"
-    repo_name = "collection_column_join"
-    repo_revision = "dfde09461b1e"
+    repo_name = "featurecounts"
+    repo_revision = "f9d49f5cb597"
 
     install_target: InstallationTarget = StandaloneInstallationTarget(tmp_path)
     install_manager = InstallRepositoryManager(install_target)
@@ -33,10 +33,22 @@ def test_against_production_shed(tmp_path: Path):
         repo_revision,  # revision 2, a known installable revision
         install_options,
     )
+    tool_guid = f"toolshed.g2.bx.psu.edu/repos/{repo_owner}/{repo_name}/featurecounts/2.0.3+galaxy2"
     with open(tmp_path / "shed_conf.xml") as f:
-        assert "toolshed.g2.bx.psu.edu/repos/iuc/collection_column_join/collection_column_join/0.0.2" in f.read()
+        assert tool_guid in f.read()
     repo_path = tmp_path / "tools" / "toolshed.g2.bx.psu.edu" / "repos" / repo_owner / repo_name / repo_revision
     assert repo_path.exists()
+    tool_data_table_path = (
+        tmp_path
+        / "tool_data"
+        / "toolshed.g2.bx.psu.edu"
+        / "repos"
+        / repo_owner
+        / repo_name
+        / repo_revision
+        / "tool_data_table_conf.xml"
+    )
+    assert tool_data_table_path.exists()
 
     install_model_context = install_target.install_model.context
     query = install_model_context.query(ToolShedRepository).where(ToolShedRepository.name == repo_name)
@@ -54,7 +66,7 @@ def test_against_production_shed(tmp_path: Path):
     assert not errors
 
     with open(tmp_path / "shed_conf.xml") as f:
-        assert "toolshed.g2.bx.psu.edu/repos/iuc/collection_column_join/collection_column_join/0.0.2" not in f.read()
+        assert tool_guid not in f.read()
 
-    repo_path = tmp_path / "tools" / "toolshed.g2.bx.psu.edu" / "repos" / repo_owner / repo_name / repo_revision
     assert not repo_path.exists()
+    # Tool data tables are not removed when uninstalling a repository