#6 Add support for merging PR repos with default configuration
Merged by merlinm. Opened by merlinm.
fedora-ci/ merlinm/cccc merge-pr-repos  into  master

Also added another small git test, moved pungi test data to a subdirectory, added config file loading, added an option to execute_cmd(), and added new command line parameters.

Seems to basically work when provided with a private config file that sets merged_repo_url, merged_repo_file, merged_repo_id_file, odcs_raw_config_name, and odcs_server according to our internal test setup.

@jkaluza Any feedback on my current approach here would be appreciated!

And a few questions I have for you...

  • Do you think I should re-do the general configuration handling so config settings can be accessed using conf.property instead of conf["property"] by copying something like what odcs does?

  • How should the new git branches be named in the merged config repo? My current scheme of using a time+date stamp is just asking for race condition trouble, and doesn't seem descriptive enough.

  • What username and e-mail addresses should be used for git commits in the merged config repo--or are the current dummy values fine? (Perhaps they should be made config settings, or additional command line parameters.)

5 new commits added

  • WIP Add support for merging PR repos with default configuration
  • utils: add ignore_fail parameter to execute_cmd()
  • Add config file loading
  • Move pungi test data to a subdirectory
  • Additional git testing

@jkalina I reorganized and cleaned up the code a bit.

In addition to my previous questions, I have another...

  • What is the relationship between the --default-pungi-file command line file name, and the configuration file name odcs is expecting based on the raw_config name it will be given? Do the file names need to be the same, or should cccc read the pungi config from the command line argument file name and write out the modified file using the file name odcs is expecting. Or something else?

I'm thinking about one thing here. The ODCS raw_config compose configuration must always include config_filename (see https://docs.pagure.org/odcs/raw_config.html#adding-the-raw-config-urls-record). It is therefore not possible to submit ODCS compose request with any random Pungi configuration filename. So for example for Fedora composes, the filename in ODCS is hardcoded to fedora.conf, for ELN composes, the filename is hardcoded to eln.conf.

I think it would save some ODCS configuration if we would just rename Pungi configuration file from the pungi_config_file to cccc.conf before committing merged_dir. That way, I could configure ODCS to always expect config_filename to be cccc.conf for CCCC raw configuration and the same configuration could be used for both fedora.conf and eln.conf (or even other future repositories/branches/configs) without the need to reconfigure ODCS.

I'm not sure if it's clear, so we can discuss it during the meeting today.

@jkaluza Any feedback on my current approach here would be appreciated!
And a few questions I have for you...

Do you think I should re-do the general configuration handling so config settings can be accessed using conf.property instead of conf["property"] by copying something like what odcs does?

I think it is not necessary. I do not have any preference here. Even the current implementation looks good.

How should the new git branches be named in the merged config repo? My current scheme of using a time+date stamp is just asking for race condition trouble, and doesn't seem descriptive enough.

I'm not sure I have better solution for now. It would be nice to somehow encode the PR ID and repository name. Maybe it could accept this as command line argument and jenkins job would set it? Because Jenkins job knows the context and it knows the PR ID and name of the project the PR is created against.

I would still fallback to timestamp in case this argument is not provided.

What do you think?

What username and e-mail addresses should be used for git commits in the merged config repo--or are the current dummy values fine? (Perhaps they should be made config settings, or additional command line parameters.)

Maybe we can change the default username from Unknown to CCCC. Or really make it configurable together with email address.

What is the relationship between the --default-pungi-file command line file name, and the configuration file name odcs is expecting based on the raw_config name it will be given? Do the file names need to be the same, or should cccc read the pungi config from the command line argument file name and write out the modified file using the file name odcs is expecting. Or something else?

I think I answered it in my previous comment.

6 new commits added

  • Add support for merging PR repos with default configuration
  • main: add new command line parameters --debug and --branch-name
  • utils: add ignore_fail parameter to execute_cmd()
  • Add config file loading
  • Move pungi test data to a subdirectory
  • Additional git testing

It looks good, +1 to merge.

Pull-Request has been merged by merlinm