[23.2] Move carbon data loading into config

[mvdbeek]

Fixes the config package that otherwise breaks on import. I don't like it, this should have probably been an external service but here we are. Maybe it'll just not work out to have a separate config package (which was created for gravity).

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[nsoranzo]

Unit test failure is legit.

[jdavcs]

I don't think the calculation of these values belonged in the config package in the first place. Of course we don't have a formal set of guidelines of what kind of computations belong in the config package and what should be computed elsewhere. We should have such guidelines, of course. Sorry I missed the initial PR - so now it's too late to discuss alternatives; besides it's not the only example of (some form of) business logic being handled in the config's almighty _process_config method. I'm approving this (it's a good enough fix for the problem at hand), but I will likely suggest an alternative sooner rather than later.
Sorry for the hand waving, but I wanted to voice my overall concerns.

[mvdbeek]

I do have an alternative commit that I did first where I just tacked it onto app:

commit 9f2d221ac087184d11f70ff45362d04aca0d9512
Author: mvdbeek <[email protected]>
Date:   Mon Dec 11 11:51:38 2023 +0100

    Move carbon data loading out of app config
    
    I don't think we want to ship this with config package and we also don't
    want to load this everytime we start e.g. a celery worker.
    This should probably also not just be something on app.

diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py
index 1c8d04847c..2cf80ff39c 100644
--- a/lib/galaxy/app.py
+++ b/lib/galaxy/app.py
@@ -33,6 +33,7 @@ from galaxy.celery.base_task import (
     GalaxyTaskBeforeStartUserRateLimitPostgres,
     GalaxyTaskBeforeStartUserRateLimitStandard,
 )
+from galaxy.carbon_emissions import get_carbon_intensity_entry
 from galaxy.config_watchers import ConfigWatchers
 from galaxy.datatypes.registry import Registry
 from galaxy.files import ConfiguredFileSources
@@ -773,6 +774,11 @@ class UniverseApplication(StructuredApp, GalaxyManagerApplication):
 
         self.interactivetool_manager = InteractiveToolManager(self)
 
+        # Carbon emissions configuration
+        carbon_intensity_entry = get_carbon_intensity_entry(self.config.geographical_server_location_code)
+        self.carbon_intensity = carbon_intensity_entry["carbon_intensity"]
+        self.geographical_server_location_name = carbon_intensity_entry["location_name"]
+
         # Configure handling of signals
         handlers = {}
         if self.heartbeat:
diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py
index a3dfa347e6..6bd8359bac 100644
--- a/lib/galaxy/config/__init__.py
+++ b/lib/galaxy/config/__init__.py
@@ -33,7 +33,6 @@ from urllib.parse import urlparse
 
 import yaml
 
-from galaxy.carbon_emissions import get_carbon_intensity_entry
 from galaxy.config.schema import AppSchema
 from galaxy.exceptions import ConfigurationError
 from galaxy.util import (
@@ -842,11 +841,6 @@ class GalaxyAppConfiguration(BaseAppConfiguration, CommonConfigurationMixin):
         else:
             self.version_extra = extra_info
 
-        # Carbon emissions configuration
-        carbon_intensity_entry = get_carbon_intensity_entry(kwargs.get("geographical_server_location_code", ""))
-        self.carbon_intensity = carbon_intensity_entry["carbon_intensity"]
-        self.geographical_server_location_name = carbon_intensity_entry["location_name"]
-
         # Database related configuration
         self.check_migrate_databases = string_as_bool(kwargs.get("check_migrate_databases", True))
         if not self.database_connection:  # Provide default if not supplied by user
diff --git a/lib/galaxy/managers/configuration.py b/lib/galaxy/managers/configuration.py
index 78b38046d8..b0b196553a 100644
--- a/lib/galaxy/managers/configuration.py
+++ b/lib/galaxy/managers/configuration.py
@@ -177,8 +177,8 @@ class ConfigSerializer(base.ModelSerializer):
             "aws_estimate": _use_config,
             "carbon_emission_estimates": _defaults_to(True),
             "carbon_intensity": _use_config,
-            "geographical_server_location_name": _use_config,
-            "geographical_server_location_code": _use_config,
+            "geographical_server_location_name": lambda item, key, *, trans, **context: trans.app.geographical_server_location_name,
+            "geographical_server_location_code": lambda item, key, *, trans, **context: trans.app.geographical_server_location_code,
             "power_usage_effectiveness": _use_config,
             "message_box_content": _use_config,
             "message_box_visible": _use_config,

if we prefer that that's also fine with me but I hate all of this and we shouldn't have merged this, as you said.

[jdavcs]

I think your alternative - tacking it onto app - is better. +1 on doing that. Also, I didn't even think of "I don't think we want to ship this with config package and we also don't want to load this everytime we start"...

Since you're moving that code out of config, do you mind fixing one little thing? This -
carbon_intensity_entry = get_carbon_intensity_entry(kwargs.get("geographical_server_location_code", ""))
should be this:
carbon_intensity_entry = get_carbon_intensity_entry(self.geographical_server_location_code) (because it's in the config schema, so it's loaded automatically, passing through any pre-processing steps we use for all schema-defined config values)