Page MenuHomeSoftware Heritage

(fix ci) search.cli: Fix journal client instantiation and add config checks
ClosedPublic

Authored by ardumont on May 5 2020, 10:22 AM.

Details

Summary

Prior to this commit, the import was no longer relevant (since journal v0.0.31).

This commit fixes the import (and bumps the journal requirements).

It also:

  • adapts journal_client instantiation with the missing brokers, group_id, etc... (as a plain import change was not enough).
  • adds configuration checks on the journal client (+ tests)

This fixes the current ci build failure [1]

[1] https://jenkins.softwareheritage.org/job/DSEA/job/tests/260/console

Test Plan

tox

Diff Detail

Repository
rDSEA Archive search
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.May 5 2020, 10:22 AM

Build is green

Patch application report for D3122 (id=11090)

Rebasing onto aa28b72c9a...

Current branch diff-target is up to date.
Changes applied before test
commit 849089a099f20fd6a8ce86776025b86805091a99
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue May 5 09:31:27 2020 +0200

    search.cli: Fix journal client instantiation and add config checks
    
    Prior to this commit, the import was wrong since a new journal release changed
    it.
    
    This commit fixes that import and bumps the journal requirements.
    
    It also:
    - adapts journal_client instantiation with the missing brokers, group_id,
    etc...
    - add configuration checks on the journal client (+ tests)

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

ardumont edited the summary of this revision. (Show Details)May 5 2020, 10:27 AM
ardumont added projects: Archive search, Journal.
ardumont retitled this revision from search.cli: Fix journal client instantiation and add config checks to (fix ci) search.cli: Fix journal client instantiation and add config checks.
ardumont edited the summary of this revision. (Show Details)
ardumont edited the summary of this revision. (Show Details)May 5 2020, 10:34 AM
vlorentz requested changes to this revision.EditedMay 5 2020, 1:14 PM

why was get_journal_client removed from swh.journal.cli?

All the code you're adding in swh/search/cli.py should be in swh-journal, so it can be used by other CLIs using a journal client

This revision now requires changes to proceed.May 5 2020, 1:14 PM

why was get_journal_client removed from swh.journal.cli?

All the code you're adding in swh/search/cli.py should be in swh-journal, so it can be used by other CLIs using a journal client

because there is (virtually) no swh.journal.cli any more.

The non-click-specific part of this get_journal_client function has been moved to swh.journal.client

vlorentz added inline comments.May 5 2020, 2:13 PM
swh/search/cli.py
68–79

remove this

70–72

replace this with **journal_cfg

ardumont updated this revision to Diff 11093.May 5 2020, 2:26 PM

Adapt according to review

I kept the configuration tests though.
Shout out if it needs to be removed...

Build is green

Patch application report for D3122 (id=11093)

Rebasing onto aa28b72c9a...

Current branch diff-target is up to date.
Changes applied before test
commit 4dbcbc4e7d1ce931b5dfdc3653ca3b5f2cf00e1b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue May 5 09:31:27 2020 +0200

    search.cli: Fix journal client instantiation and add config checks
    
    Prior to this commit, the import was wrong since a new journal release changed
    it.
    
    This commit fixes that import and bumps the journal requirements.
    
    It also:
    - adapts journal_client instantiation with the missing brokers, group_id,
    etc...
    - add configuration checks on the journal client (+ tests)

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

vlorentz accepted this revision.May 5 2020, 2:31 PM
This revision is now accepted and ready to land.May 5 2020, 2:31 PM