Kayode local yaml config dev #2646

kayodegigz · 2025-08-21T04:56:08Z

TODO:

Write new tests or update the old ones to cover new functionality.
Update doc-strings where appropriate.
Update or write new documentation in packit/packit.dev.
‹fill in›

Fixes

Related to

Merge before/after

RELEASE NOTES BEGIN

Packit now supports automatic ordering of ☕ after all checks pass.

RELEASE NOTES END

softwarefactory-project-zuul · 2025-08-21T05:19:19Z

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f7f4e3387f3c42eb868c528e4bd03178

✔️ pre-commit SUCCESS in 2m 01s
✔️ packit-tests-rpm SUCCESS in 22m 09s
✔️ packit-tests-pip-deps SUCCESS in 22m 17s
✔️ packit-tests-git-main SUCCESS in 22m 30s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 39s
✔️ packit-tests-git-main-sess-rec SUCCESS in 3m 01s
✔️ reverse-dep-packit-service-tests SUCCESS in 5m 09s

LecrisUT

On a general note, I would recommend to view PRs not as a complete implementation of an issue, and try to implement everything from the start, particularly if you are not familiar with all the intricacies of a project. Instead try to break it down into more minimal pieces and evolve it together with the reviewer comments, and make comments whenever you are uncertain about an issue or design.

And also I believe you are a GSOC or outreachy or something along those lines. I would recommend to make use of the opportunity to learn how to navigate an open source project in general, how to approach resolving issues, etc. If you have questions, take the opportunity to ask the mentors and people in the community directly, instead of LLM tools.

LecrisUT · 2025-08-25T15:40:45Z

packit/constants.py


+PROJECT_LOCAL_CONFIG_FILE = ".packitLocal.yaml"
+
+ALLOWED_LOCAL_OVERRIDES = {"debug", "fas_user", "fas_password", "plans"}


I wouldn't add any of the keys available in the User config file, until there appears to be a need for it. These are also a completely different schema to the project config files which would make the implementation quite complicated.

LecrisUT · 2025-08-25T15:47:23Z

packit/constants.py

    "packit.yml",
 }

+PROJECT_LOCAL_CONFIG_FILE = ".packitLocal.yaml"


See the configuration for the CONFIG_FILE_NAMES, it should be able to handle the different naming schemas as above. I would try instead to reuse the logic of that variable and alter the path by adding a suffix to the filenames being tested. See packit.config.package_config.find_packit_yaml.

LecrisUT · 2025-08-25T15:52:40Z

packit/config/config.py

+        project_local_config = cls.get_project_local_config()
+        if project_local_config:
+            validated_config = cls._validate_project_local_config(project_local_config)
+            loaded_config = cls._merge_configs(loaded_config, validated_config)
+            logger.debug("Merged project and local yaml config overrides")


This is not where we would want to alter. This would allow user-configs to be overridden, but we would rather want the project-configs to be overridden instead. That is located in

packit/packit/config/package_config.py

Lines 285 to 289 in fdcdf3a

def find_packit_yaml(

*directory: Union[Path, str],

try_local_dir_first: bool = False,

try_local_dir_last: bool = False,

) -> Path:

This of course also depends a lot on what is the usecase of #1910 and we should figure out in the issue.

Hi @LecrisUT , looking at this a second time, I don't think we would want to alter here. Reason being that the method is used for finding the packit.yaml file, I think we should separate the logic of finding the local user config. I think the change should be made here instead:

packit/packit/config/package_config.py

Line 382 in fdcdf3a

def get_local_package_config(

So we find and load the two configs separately, then merge them here before final parsing

Yes and no. Yes, the logic of getting the local/user config file would be outside at the caller of find_packit_yaml. But, no, you would want to alter that function, because that is the part where the various CONFIG_FILE_NAMES are being tested, and you would want to have the same logic used, except for appending/prepending something to the file name.

I don't quite understand. The logic of trying various CONFIG_FILE_NAMES is at the point of getting the user config right? That's what the find_packit_yaml method does. And we agree that the logic of getting the local user config should be separate. If these are true then I don't see the need for a change to this method

The logic of trying various CONFIG_FILE_NAMES is at the point of getting the user config right?

It is not specific to user config, project config uses it also ¹. There is package_config.find_packit_yaml and Config.get_user_config.

Footnotes

https://github.com/search?q=repo%3Apackit%2Fpackit%20CONFIG_FILE_NAMES&type=code ↩

Yes I agree with you, but in the case of this local user config that we're implementing here, the CONFIG_FILE_NAMES would change to something like:

LOCAL_USER_CONFIG_FILE_NAMES = { ".packitLocal.yaml", ".packitLocal.yml", "packitLocal.yaml", "packitLocal.yml", }

That's why I think it should be separate.

Or I could just introduce a flag into the find_packit_yaml method argument that would be used to specifically check for only packitLocal.yaml files if that is the case, what do you think?

Or I could just introduce a flag into the find_packit_yaml method argument that would be used to specifically check for only packitLocal.yaml files if that is the case, what do you think?

Yes, that was what I was referring to (specifically a kwarg where you would pass "Local" or otherwise be a default ""). Specifically, you just need to alter config_file_name(_full) in

packit/packit/config/package_config.py

Lines 329 to 330 in fdcdf3a

for config_file_name in CONFIG_FILE_NAMES:

config_file_name_full = config_dir / config_file_name

Here's a snippet to get you inspired

>>> from pathlib import Path >>> path = Path("/path/to/.abc.def") >>> path.with_name(f"{path.stem}XYZ{path.suffix}") PosixPath('/path/to/.abcXYZ.def')

I don't have any preference on how to tackle this, you can use the string directly to manipulate the file name

>>> ".abc.def".rsplit(".",1) ['.abc', 'def']

Alright then, thank you

LecrisUT · 2025-08-25T15:55:54Z

packit/config/config.py

        return Proxy(partial(self._get_project, url, required, get_project_kwargs))

+    @classmethod
+    def get_project_local_config(cls) -> dict:


All of these I don't think are transferable to packit.config.package_config.find_packit_yaml. You might want to re-implement some of it, but it would be somewhere around the caller part.

LecrisUT · 2025-08-25T16:02:22Z

packit/cli/test.py

+@click.option(
+    "--clean-before/--no-clean-before",
+    default=False,
+    help="Run 'tmt clean --all' before starting the test (default: disabled).",
+)


I did not check what the discussions were during the implementation of this feature, I might need to give it a review. Is it all in #2632?

This option specifically, I do not see a reason to expose, and it is a different PR to be discussed.

@kayodegigz you need to rebase your PR. Most of the Lecris comments are related with your PR being outdated (or based on the wrong branch).

Hi @majamassarini , I have rebased

LecrisUT · 2025-08-25T16:05:42Z

packit/cli/init_local_yaml.py

+@click.command(
+    "init-local-config",
+    context_settings=get_context_settings(),
+    short_help="Creates a localyaml config file for and ensures it's not version controlled",
+)
+@pass_config
+def init_local_yaml(config):


As mentioned in the issue, let's deffer this.

LecrisUT · 2025-08-25T16:06:30Z

packit/api.py

        target: Optional[str] = "fedora:rawhide",
-        rpm_paths: Optional[list[str]] = None,
-        run_all: bool = False,
+        rpm_paths: Optional[list[Path]] = None,


Same comment as in the cli counterpart of this.

LecrisUT · 2025-08-25T16:12:07Z

packit/utils/local_test_utils.py

+class LocalTestUtils:
+    @staticmethod
+    def _build_tmt_cmd(


Probably part of the other comments about splitting this feature to a different PR.

But to quickly note something, the LocalTestUtils has no meaning when all the functions are static. It should be plain functions instead. Use classes if:

You want to inherit and override some implementations (in which case classmethod is a more pythonic elegant way of designing it)

You need to instantiate an object with different parameters (in which case you use plain methods)

LecrisUT · 2025-08-25T16:15:34Z

tests/unit/test_local_test_utils.py

+class TestBuildTmtCmd:
+    """Test cases for _build_tmt_cmd function"""
+
+    def test_basic_command_structure(self):


Afaict, packit doesn't use class-based organization of tests. Use fixtures and plain function-based tests unless instructed otherwise by the core packit team.

LecrisUT · 2025-08-25T16:16:08Z

tests/unit/test_local_test_utils.py

+# SPDX-License-Identifier: MIT
+
+from pathlib import Path
+from unittest.mock import patch


packit uses pytest not unittest. See what fixtures are available either from the packit tests, pytest directly, or any other dependencies that are being used. It might be faster to state how you need to prepare the more complicated tests and then we can point you to some fixtures that you can try to use.

Do not rely on any suggestions from something like an LLM. It would most likely re-implement a lot of things that are already present in the code. Just ask us about it, we would be slower at giving you an answer, but we would be giving you that answer at review time anyway, so it would be a lot faster if you ask any of us directly, maybe as a quick question in the matrix room.

softwarefactory-project-zuul · 2025-09-04T11:15:00Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/e16131b68b1e468b844c86c41c69dce8

✔️ pre-commit SUCCESS in 2m 17s
❌ packit-tests-rpm FAILURE in 22m 47s
❌ packit-tests-pip-deps FAILURE in 22m 44s
❌ packit-tests-git-main FAILURE in 22m 54s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 44s
✔️ packit-tests-git-main-sess-rec SUCCESS in 3m 03s
✔️ reverse-dep-packit-service-tests SUCCESS in 5m 25s

mfocko · 2025-09-10T21:09:17Z

packit/cli/init_local_yaml.py

+@pass_config
+def init_local_yaml(config):
+    """Initialize packit-local yaml configuration file."""
+    config_path = Path(".packitLocal.yaml")


I'm not sure how I feel about the config name, but I guess that can be easily adjusted.

Also I don't have a good suggestion for naming it, not to mention how convoluted this is becoming.

mfocko · 2025-09-10T21:10:35Z

packit/cli/init_local_yaml.py

+        # Authentication (optional - overrides ~/.config/packit.yaml)
+        # authentication:
+        #   github.com:
+        #     token: YOUR_TOKEN


I don't think that this is the exact goal of this feature, and also I have a feeling that if you go in this direction, you might complicate this issue by a lot for yourself.

If I understood correctly, the aim was to provide a way to configure Packit jobs that can be run only locally.

mfocko · 2025-09-10T21:11:50Z

packit/cli/init_local_yaml.py

+    else:
+        with open(gitignore_path, "w") as f:
+            f.write(f"# Packit local config (local only overrides)\n{localYamlFile}\n")
+        click.echo(f"Created .gitignore with {localYamlFile}")


Careful here, this branch doesn't imply that the current working directory is even version-controlled.

Hi @mfocko , we have stalled this particular implementation to auto generate a yaml file, as advised by @LecrisUT

mfocko · 2025-09-10T21:14:24Z

packit/cli/init_local_yaml.py

+# # Copyright Contributors to the Packit project.
+# # SPDX-License-Identifier: MIT


Why are you commenting out even the comments?

Why are you even commenting it out?

mfocko · 2025-09-10T21:15:26Z

packit/config/config.py

-                    f"Allowed keys are: {', '.join(sorted(ALLOWED_LOCAL_OVERRIDES))}",
-                )
-        return validated
+    # @classmethod


Just an FYI, as long as you don't alter the history in the git, you are able to recover any changes, i.e., there's no need to comment out code, if you don't need it, just delete it.

mfocko · 2025-09-10T21:16:11Z

packit/config/package_config.py

+            *directory,
+            try_local_dir_first=True,
+            try_local_dir_last=False,
+            suffix="Local",


I have a feeling that this is a pretty good candidate for extending the part of the user config, so it's possible to configure this. You have prepared it pretty well for that.

mfocko · 2025-09-10T21:16:56Z

packit/config/package_config.py

+        local_loaded_config = load_packit_yaml(config_file_path=local_config_file_name)
+        local_loaded_config = validate_project_local_config(local_loaded_config)


These two lines don't make much sense type-wise, i.e., they are correct, Python allows it, but it doesn't read very well.

Hmmm you think I should just assign to another variable altogether? I did that because the second method would also return a dict, just a cleaned up one.

mfocko · 2025-09-10T21:18:53Z

packit/config/package_config.py

+def merge_configs(base: dict, override: dict) -> dict:
+    """
+    Deep merge override config into base config.
+    Override values take precedence.
+    """
+    from copy import deepcopy
+
+    result = deepcopy(base)
+
+    for key, value in override.items():
+        if key in result and isinstance(result[key], dict) and isinstance(value, dict):
+            result[key] = merge_configs(result[key], value)
+        else:
+            result[key] = value
+
+    return result


This definitely needs to be covered by tests. Not for the sake of testing, but for the sake of defining the expected behavior.

mfocko · 2025-09-10T21:19:48Z

packit/constants.py

+LOCAL_USER_CONFIG_FILE_NAMES = {
+    ".packitLocal.yaml",
+    ".packitLocal.yml",
+    "packitLocal.yaml",
+    "packitLocal.yml",
+}


I don't really understand why do you hard-code the filenames here, but at the same time you allow the “middle” part to be customized as part of the interface.

This was an oversight, I forgot to remove, sorry.

-- Related to packit#1910

softwarefactory-project-zuul · 2025-09-20T15:02:51Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f0ddc6e6300a4004933d8ee24787d3d4

✔️ pre-commit SUCCESS in 2m 01s
❌ packit-tests-rpm FAILURE in 22m 30s
❌ packit-tests-pip-deps FAILURE in 22m 29s
❌ packit-tests-git-main FAILURE in 22m 43s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 38s
✔️ packit-tests-git-main-sess-rec SUCCESS in 2m 52s
❌ reverse-dep-packit-service-tests FAILURE in 5m 15s

softwarefactory-project-zuul · 2025-09-20T15:47:38Z

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/29a3b532a14445098ccdf7e472bcb2cc

✔️ pre-commit SUCCESS in 2m 01s
❌ packit-tests-rpm FAILURE in 22m 06s
❌ packit-tests-pip-deps FAILURE in 22m 14s
❌ packit-tests-git-main FAILURE in 22m 37s
✔️ packit-tests-pip-deps-sess-rec SUCCESS in 2m 39s
✔️ packit-tests-git-main-sess-rec SUCCESS in 2m 59s
❌ reverse-dep-packit-service-tests FAILURE in 5m 14s

lbarcziova · 2026-02-02T11:19:24Z

hi @kayodegigz! Do you plan to continue working on this?

kayodegigz mentioned this pull request Aug 21, 2025

Project-User configuration file #1910

Open

LecrisUT suggested changes Aug 25, 2025

View reviewed changes

mfocko reviewed Sep 10, 2025

View reviewed changes

kayodegigz added 2 commits September 20, 2025 15:37

implement first stage packit local config

53164de

-- Related to packit#1910

second review from fixes

4651848

kayodegigz force-pushed the kayode-local-yaml-config-dev branch from 3fac935 to 4651848 Compare September 20, 2025 14:39

changes from review

2e16b3f


		PROJECT_LOCAL_CONFIG_FILE = ".packitLocal.yaml"

		ALLOWED_LOCAL_OVERRIDES = {"debug", "fas_user", "fas_password", "plans"}

	def find_packit_yaml(
	*directory: Union[Path, str],
	try_local_dir_first: bool = False,
	try_local_dir_last: bool = False,
	) -> Path:

	for config_file_name in CONFIG_FILE_NAMES:
	config_file_name_full = config_dir / config_file_name

		# # Copyright Contributors to the Packit project.
		# # SPDX-License-Identifier: MIT

		local_loaded_config = load_packit_yaml(config_file_path=local_config_file_name)
		local_loaded_config = validate_project_local_config(local_loaded_config)

Kayode local yaml config dev #2646

Are you sure you want to change the base?

Kayode local yaml config dev #2646

Conversation

kayodegigz commented Aug 21, 2025

Uh oh!

softwarefactory-project-zuul bot commented Aug 21, 2025

Uh oh!

LecrisUT left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LecrisUT Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

majamassarini Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

softwarefactory-project-zuul bot commented Sep 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LecrisUT Aug 28, 2025 •

edited

Loading

majamassarini Sep 11, 2025 •

edited

Loading