Page MenuHomeSoftware Heritage

data/defaults: Drop global.ini file
ClosedPublic

Authored by ardumont on Wed, Sep 16, 12:45 PM.

Details

Summary

This should allow dropping "dead code" in swh.core.config module regarding ini
file [1]. This is the remaining sole instance of such file.

[1] https://forge.softwareheritage.org/source/swh-core/browse/master/swh/core/config.py$96-104

Related to T1532

Test Plan

octocatalog

bin/octocatalog-diff --octocatalog-diff-args --no-truncate-details --to staging uffizi
Found host uffizi.softwareheritage.org
Cloning into '/tmp/swh-ocd.WIAYt237/environments/production/data/private'...
done.
Cloning into '/tmp/swh-ocd.WIAYt237/environments/staging/data/private'...
done.
*** Running octocatalog-diff on host uffizi.softwareheritage.org
I, [2020-09-16T14:27:38.255079 #24752]  INFO -- : Catalogs compiled for uffizi.softwareheritage.org
I, [2020-09-16T14:27:39.467942 #24752]  INFO -- : Diffs computed for uffizi.softwareheritage.org
diff origin/production/uffizi.softwareheritage.org current/uffizi.softwareheritage.org
*******************************************
  File[/etc/softwareheritage/global.ini] =>
   parameters =>
     ensure =>
      - file
      + absent
*******************************************
*** End octocatalog-diff on uffizi.softwareheritage.org

Diff Detail

Repository
rSPSITE puppet-swh-site
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Wed, Sep 16, 12:45 PM
ardumont updated this revision to Diff 13947.Wed, Sep 16, 12:48 PM

Rework commit message (wrong task reference number ¯\_(ツ)_/¯)

ardumont edited the summary of this revision. (Show Details)Wed, Sep 16, 1:37 PM
olasd requested changes to this revision.Wed, Sep 16, 1:53 PM
olasd added a subscriber: olasd.

I agree with the switch to yaml, but I don't think we should have plain text yaml files in the hiera variables.

We should switch the swh::global_conf hiera variable to be a yaml dict, and the way the file is generated by the profile::swh to inline this yaml (with a "warning" header).

Or we could drop the file altogether, as it's not being used at all now.

This revision now requires changes to proceed.Wed, Sep 16, 1:53 PM
olasd added a comment.Wed, Sep 16, 1:54 PM
In D3963#97665, @olasd wrote:

I agree with the switch to yaml, but I don't think we should have plain text yaml files in the hiera variables.

We should switch the swh::global_conf hiera variable to be a yaml dict, and the way the file is generated by the profile::swh to inline this yaml (with a "warning" header).

Or we could drop the file altogether, as it's not being used at all now.

We should also make profile::swh remove the old file, because it will still be loaded if it exists, even if it's not referenced in puppet.

ardumont added a comment.EditedWed, Sep 16, 2:12 PM

Or we could drop the file altogether, as it's not being used at all now.

yes, thanks for confirming this.
I'll get rid of it then...

We should also make profile::swh remove the old file, because it will still be loaded if it exists, even if it's not referenced in puppet.

...properly indeed ;)

I intend to follow up on the code cleanup after that.

ardumont updated this revision to Diff 13952.Wed, Sep 16, 2:27 PM

Drop variable in the end

Note: don't worry about the commits, i'll squash them.
For now, i'm making sure we agree on the perimeter and the implementation ;)

ardumont retitled this revision from data/defaults: Modify global.ini to global.yml to data/defaults: Drop global.ini file.Wed, Sep 16, 2:28 PM
ardumont edited the test plan for this revision. (Show Details)
ardumont updated this revision to Diff 13954.Wed, Sep 16, 2:44 PM

Update diffs with the 2 commits i'm talking about (i think i messed up the prior update)

olasd accepted this revision.Wed, Sep 16, 2:45 PM

Bye bye .ini.

log_db should also be removed from the swh.core.config default config. the PostgreSQL logger has been removed a long time ago but it seems this has been missed.

This revision is now accepted and ready to land.Wed, Sep 16, 2:45 PM
ardumont updated this revision to Diff 13964.EditedWed, Sep 16, 4:01 PM

squash and rework commit message

fwiw, i've checked on staging that loaders are still properly starting without
it. This logs one less message about that mostly empty file \m/.

This revision was automatically updated to reflect the committed changes.