Paths

Table of Contentst

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

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

Details

Reviewers

vlorentz
douardda

Group Reviewers

Reviewers

Commits

rDSEA4dbcbc4e7d1c: search.cli: Fix journal client instantiation and add config checks

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

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 12263
Build 18598: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 18597: arc lint + arc unit

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.

Harbormaster completed remote builds in B12263: Diff 11090.May 5 2020, 10:23 AM

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

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

In D3122#75850, @vlorentz wrote:

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
83–85	replace this with `**journal_cfg`

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.

Harbormaster completed remote builds in B12266: Diff 11093.May 5 2020, 2:27 PM

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

Closed by commit rDSEA4dbcbc4e7d1c: search.cli: Fix journal client instantiation and add config checks (authored by ardumont). · Explain WhyMay 5 2020, 2:33 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDSEA4dbcbc4e7d1c: search.cli: Fix journal client instantiation and add config checks.