Skip to content
Snippets Groups Projects

publisher: Simplify publisher initialization and add journal cli

Closed publisher: Simplify publisher initialization and add journal cli
generated-differential-D1249-source into generated-differential-D1249-target
3 unresolved threads
Closed Antoine R. Dumont requested to merge generated-differential-D1249-source into generated-differential-D1249-target
3 unresolved threads

This allows to pass the configuration needed to the publisher.

I extracted that from the current work i'm doing on testing kafka. That allows to unblock other coming changes needed (impact on swh-docker-dev, swh-site).

Related swh-core#1410

Test Plan

tox

Migrated from D1249 (view on Phabricator)

Merge request reports

Approved by
vlorentz

Closed by Phabricator Migration userPhabricator Migration user 6 years ago (Mar 15, 2019 12:50pm UTC)

Merge details

  • The changes were not merged into generated-differential-D1249-target.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
swh/journal/tests/test_publisher.py
139 145
140 146 class TestPublisher(unittest.TestCase):
141 147 def setUp(self):
142 self.publisher = JournalPublisherTest()
148 self.publisher = JournalPublisherNoKafkaInMemoryStorage(TEST_CONFIG)
  • vlorentz
    vlorentz @vlorentz started a thread on the diff
    • swh/journal/cli.py 0 → 100644
    62 'publisher_id', 'object_types', 'storage'
    63 ]
    64
    65 conf = ctx.obj['config']
    66 missing_keys = []
    67 for key in mandatory_keys:
    68 if not conf.get(key):
    69 missing_keys.append(key)
    70
    71 if missing_keys:
    72 raise click.ClickException(
    73 'Configuration error: The following keys must be'
    74 ' provided: %s' % (','.join(missing_keys), ))
    75
    76 publisher = JournalPublisher(conf)
    77 try:
  • vlorentz mentioned in merge request swh-docker-dev!129 (closed)

    mentioned in merge request swh-docker-dev!129 (closed)

  • vlorentz mentioned in merge request !242 (closed)

    mentioned in merge request !242 (closed)

  • vlorentz
    vlorentz @vlorentz ·
    Maintainer

    Accepted, but let's not forget to add tests for the CLI

  • vlorentz
    vlorentz @vlorentz ·
    Maintainer

    Merge request was accepted

  • vlorentz approved this merge request

    approved this merge request

  • vlorentz
    vlorentz @vlorentz started a thread on the diff
    • swh/journal/cli.py 0 → 100644
    58
    59 """
    60 mandatory_keys = [
    61 'brokers', 'temporary_prefix', 'final_prefix', 'consumer_id',
    62 'publisher_id', 'object_types', 'storage'
    63 ]
    64
    65 conf = ctx.obj['config']
    66 missing_keys = []
    67 for key in mandatory_keys:
    68 if not conf.get(key):
    69 missing_keys.append(key)
    70
    71 if missing_keys:
    72 raise click.ClickException(
    73 'Configuration error: The following keys must be'
  • Phabricator Migration user mentioned in commit swh/infra/puppet/puppet-swh-site@01897647

    mentioned in commit swh/infra/puppet/puppet-swh-site@01897647

  • Antoine R. Dumont
    Antoine R. Dumont @ardumont ·
    Author Maintainer

    Thanks for the remarks.

    Yes, for the test on cli. I planned to do that but not right now. My main focus right now is being able to test the kafka layer for now ;)

  • Antoine R. Dumont
    Antoine R. Dumont @ardumont ·
    Author Maintainer

    Adapt according to review

  • Software Heritage Continuous Integration for public repositories
    Software Heritage Continuous Integration for public repositories @swh-public-ci ·

    Build is green See https://jenkins.softwareheritage.org/job/DJNL/job/tox/37/ for more details.

  • Antoine R. Dumont
    Antoine R. Dumont @ardumont ·
    Author Maintainer

    Merge request was merged

  • Antoine R. Dumont closed

    closed

    • Please register or sign in to reply