Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add global config reasearch #220

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

majamassarini
Copy link
Member

@majamassarini majamassarini commented Dec 19, 2024

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks Maja for the detailed write-down with the examples! I just quickly went through them, and the approaches you provided look to me all like some templating mechanisms in the end, or? Could you please also cover something like Zuul config for Fedora dist-git (see), i.e. a config in one place that is edited by users there?


Splitting the configuration in multiple configuration files will lead obviously to worst performance. Personally I don't see a way to prevent it.

We can limit the number of recursion steps; 3/4 steps are, from my point of view, more than enough. Having a recursion limit will avoid an infinite recursion for malformed configurations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, we could even start with 1/2 levels

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can easily happen when copy-pasting examples. But we can also save a list of config files through the calls and check for the recursion.

Copy link
Member

@lbarcziova lbarcziova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading the whole research, I agree with you that the bottom-up approach as described currently here is definitely a cleaner and more elegant way to handle this, providing more advantages.

Although, I can still see a slightly "combined" option working, especially for Packit as the default Fedora CI use case. This could involve having one global configuration file that doesn’t include any project-specific settings—just a base configuration that could be used as-is. And (also depending on discussions and agreements with the Fedora folks and how the change proposal will be accepted), packages might only need to include an empty config file in their repository or, perhaps, no config file at all (for the final stage of the proposal), with the global configuration acting as the default. If modifications are required, specific options could then be overridden in a dedicated config file within the package repository, using some of the options in the bottom-up part of the research.

- It is probably easier, for the final user, to look for the service configuration file in its own repo instead of a service repo.
- Less user engagement due to limited visibility of configuration changes. Even though a configuration migration can be simpler for the service team it could be less explicit for the final user. Being able to change behaviour without the user acknowledging it could not be a good idea. In packit there already is a configuration migration script.
- The global configuration file could really grow huge and be difficult to maintain both for the service team and for a final user that wants to contribute to it.
- Lacks ecosystem-specific configuration management. There is no encapsulation for "middle layer" knowledge, no easy way to manage ecosystem configurations. Configurations could be grouped in different nodes with different defaults but there is no easy way, for the service team, to know if the user who is asking to update a configuration has the rights for doing so.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a good point

- Users don't need configuration files in their projects; they might not like the idea of having a service related file in their projects.
- Configuration migrations are straightforward for the service team; it is just a change in a file that belongs to the team.
- Highest performance due to direct configuration access; no need to load and pre-process other files.
- Quickest implementation; this approach is the same as for the packit-service configuration file.
Copy link
Member

@lachmanfrantisek lachmanfrantisek Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this reason, we can use this to enable dist-git CI for packages before it is run by default. (And for nothing else.)

Copy link
Member

@lachmanfrantisek lachmanfrantisek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nicely written Maja!

## Advantages

- User-friendly and explicit. Everything is on the user side (more quick for him) and the file will never grow too much because of other project details.
- Flexible customization through layered configurations and better ecosystem-specific configuration support.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big 👍 for this. It would be really nice to get SIGs help in this. And maybe create a shared space for putting these together.

### User-Side Layer

At the moment the packit service needs just one configuration file on the user project side both for the **upstream continuous integration** and for the **release synchronization**.
When implementing the **Fedora CI** (which is a _downstream continuous integration_) a new configuration file could be required or the configuration can be merged with the existing one.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just need to be careful not to introduce too many file names because of the forge API issues (but Forgejo might handle these more easily).


### Implementation

Personally I find the _global config + overlays_ approach the best and in this case we would need to:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looking at the examples (thanks for them), this one looks the most readable.


Splitting the configuration in multiple configuration files will lead obviously to worst performance. Personally I don't see a way to prevent it.

We can limit the number of recursion steps; 3/4 steps are, from my point of view, more than enough. Having a recursion limit will avoid an infinite recursion for malformed configurations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can easily happen when copy-pasting examples. But we can also save a list of config files through the calls and check for the recursion.

Comment on lines +343 to +344
<!--
We already have a configuration file that enables packit we can still count on it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this allows multi-level approach, we can have something already for the Packit-owned configs. Similarly to your example above.

  • packit-base (just to have something shared for all the packages if we need to influence anything. Maybe loaded by default for all the packages?)
  • packit-usecase (upstream-ci, downstream-ci, fedora-automation), or packit-instance

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at these Packit-owned ones. What do you think about supporting multiple base configs?

This might be handy to combine e.g. type of package (.eg. ecosystem-specific actions) with jobs setup (e.g.upstream ci).
If there is a list of base configs, we can just apply them one by one. But I get this might introduce too much mess into the system.

Comment on lines +360 to +361
- it should be visible which is the result of applying the user packit config to a packit instance config. So we should probably have a command line or a packit service command to show the user the resulting configuration.
-->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With any solution, we should provide an easy way to get an expanded configuration. Locally, but maybe also in the logs with clear references where these external configs came from.

Plus add packit 1.0.0 release experience notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants