Repository: kedro-org/kedro
File: kedro/config/omegaconf_config.py (Python)

Code before:
            "application/x-zip-compressed",
            "application/zip-compressed",
        ):
            protocol = "zip"
        else:
            protocol = "file"
        fs = fsspec.filesystem(protocol=protocol, fo=conf_source)
        return fs, protocol

    def _merge_configs(
        self,
        config: dict[str, Any],
        env_config: dict[str, Any],
        key: str,
        env_path: str,
    ) -> Any:
        merging_strategy = self.merge_strategy.get(key, "destructive")
        if merging_strategy == "soft":
            return self._soft_merge(config, env_config)
        elif merging_strategy == "destructive":
            return self._destructive_merge(config, env_config, env_path)
        else:
            raise ValueError(
                f"Merging strategy {merging_strategy} not supported. The accepted merging "
                f"strategies are `soft` and `destructive`."
            )

    def _is_valid_config_path(self, path: Path) -> bool:
        """Check if given path is a file path and file type is yaml or json."""
        posix_path = path.as_posix()
        return self._fs.isfile(str(posix_path)) and path.suffix in [
            ".yml",
            ".yaml",
            ".json",
        ]

    def _register_globals_resolver(self) -> None:
        """Register the globals resolver"""
        OmegaConf.register_new_resolver(
            "globals",
            self._get_globals_value,
            replace=True,
        )

    def _register_runtime_params_resolver(self) -> None:
        OmegaConf.register_new_resolver(
            "runtime_params",
            self._get_runtime_value,
            replace=True,
        )


Review comment (suggestion):
Shall we go even further and move merging strategies outside to the `Enum` and create a mapping of symbolic names to the implementations? Then all the possible values and implementations will be in one place, so it's easier to navigate and compare them, and additionally, we will get rid `if else` logic when applying a merging strategy. 

Code after:
    def _initialise_filesystem_and_protocol(
        conf_source: str,
    ) -> tuple[fsspec.AbstractFileSystem, str]:
        """Set up the file system based on the file type detected in conf_source."""
        file_mimetype, _ = mimetypes.guess_type(conf_source)
        if file_mimetype == "application/x-tar":
            protocol = "tar"
        elif file_mimetype in (
            "application/zip",
            "application/x-zip-compressed",
            "application/zip-compressed",
        ):
            protocol = "zip"
        else:
            protocol = "file"
        fs = fsspec.filesystem(protocol=protocol, fo=conf_source)
        return fs, protocol

    def _merge_configs(
        self,
        config: dict[str, Any],
        env_config: dict[str, Any],
        key: str,
        env_path: str,
    ) -> Any:
        merging_strategy = self.merge_strategy.get(key, "destructive")
        try:
            strategy = MergeStrategies[merging_strategy.upper()]

            # Get the corresponding merge function and call it
            merge_function_name = MERGING_IMPLEMENTATIONS[strategy]
            merge_function = getattr(self, merge_function_name)
            return merge_function(config, env_config, env_path)
        except KeyError:
            allowed_strategies = [strategy.name.lower() for strategy in MergeStrategies]
            raise ValueError(
                f"Merging strategy {merging_strategy} not supported. The accepted merging "
                f"strategies are {allowed_strategies}."
            )

    def _get_all_keys(self, cfg: Any, parent_key: str = "") -> set[str]:
        keys: set[str] = set()

        for key, value in cfg.items():
            full_key = f"{parent_key}.{key}" if parent_key else key
            if isinstance(value, dict):
                keys.update(self._get_all_keys(value, full_key))
            else:
                keys.add(full_key)
        return keys