Page MenuHomeSoftware Heritage

conf/deposit: Explicit deposit api configuration
ClosedPublic

Authored by ardumont on Sep 16 2020, 6:46 PM.

Details

Summary

This just explicits what used to be set by default in the deposit regarding the provider.
Information which is transited to the deposit loader.

Test Plan

octocatalog

bin/octocatalog-diff --octocatalog-diff-args --no-truncate-details --to staging deposit.internal.staging.swh.network
Found host deposit.internal.staging.swh.network
Cloning into '/tmp/swh-ocd.BasCM0H9/environments/production/data/private'...
done.
Cloning into '/tmp/swh-ocd.BasCM0H9/environments/staging/data/private'...
done.
*** Running octocatalog-diff on host deposit.internal.staging.swh.network
I, [2020-09-16T18:47:00.767738 #3745]  INFO -- : Catalogs compiled for deposit.internal.staging.swh.network
I, [2020-09-16T18:47:03.118164 #3745]  INFO -- : Diffs computed for deposit.internal.staging.swh.network
diff origin/production/deposit.internal.staging.swh.network current/deposit.internal.staging.swh.network
*******************************************
  File[/etc/softwareheritage/deposit/server.yml] =>
   parameters =>
     content =>
      @@ -6,4 +6,7 @@
         configuration:
           sword_version: 2
      +provider:
      +  provider_type: deposit_client
      +  metadata: {}
       scheduler:
         cls: remote
*******************************************
*** End octocatalog-diff on deposit.internal.staging.swh.network

Diff Detail

Event Timeline

ardumont retitled this revision from conf/deposit: Explicit api configuration to conf/deposit: Explicit deposit api configuration.Sep 17 2020, 8:57 AM
ardumont added inline comments.
data/defaults.yaml
1678 ↗(On Diff #13989)

wondering about that now...
This should probably dynamically set by the deposit (which would introspect its current version in some ways)
@vlorentz ^

In the other diff, i put the current last 0.0.90 tag...
That won't ease maintenance.

data/defaults.yaml
1678 ↗(On Diff #13989)

yes, I completely agree. But there was some opposition when I suggested doing that for indexer tools, so I didn't mention it.

Though now I checked the reason, and it was only for external tools, not SWH modules. https://forge.softwareheritage.org/T1464#26934

And we're already doing auto-detection in loaders, anyway

I just grepped through swh-deposit, and I can't find where this provider dict is used, other than in the config declaration. Do we really need it?

data/defaults.yaml
1678 ↗(On Diff #13989)

Thanks for confirming this and the link ;)
That should not prevent this from landing though.

I'll check on how to properly do that deposit side.
Cheers,

I just grepped through swh-deposit, and I can't find where this provider dict is used, other than in the config declaration. Do we really need it?

Yes [1]

You did not find it because it so happens it just has been removed from the "implicit" configuration loading [2]

I'm trying to improve on that... albeit slowly.

[1] https://forge.softwareheritage.org/source/swh-deposit/browse/master/swh/deposit/api/private/deposit_read.py$108-109

[2] https://forge.softwareheritage.org/D3953

data/defaults.yaml
1678 ↗(On Diff #13989)

I'll check on how to properly do that deposit side.

D3973

Build is green

Patch application report for D3969 (id=14022)

Rebasing onto 28d61c8cf1...

Current branch diff-target is up to date.
Changes applied before test
commit 4658518084d4bbe769afcdcf87290f03319a5fe9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Sep 16 15:25:52 2020 +0200

    swh.core.config: Simplify SWHConfig mixin to its actual use
    
    Related to T1532

See https://jenkins.softwareheritage.org/job/DCORE/job/tests-on-diff/61/ for more details.

This revision was not accepted when it landed; it landed in state Needs Review.Sep 18 2020, 1:58 PM
This revision was automatically updated to reflect the committed changes.