Page MenuHomeSoftware Heritage
Differential D1441

tutorial: How to run a new lister (within docker-dev)
ClosedPublic
Actions

Authored by nahimilega on May 3 2019, 10:23 AM.

Details

Reviewers
ardumont
vlorentz
Group Reviewers
Reviewers
Commits
rDLS8d1b5d2d2d92: Add new page "run_a new_lister"
rDLSb2c6ddc35b6b: tutorial: Add testing and how to run section
Summary

Added a note informing about adding the name of new lister in the main conftest.py in order to make it functional

As there is no documentation of the procedure to run lister with docker
we can extend the approach of this diff to -

  • Include how to mount your local development modifications (docker-compose.override.yml).
  • Add a section titled "How to run a *new* lister"

Diff Detail

Repository
rDLS Listers
Branch
ddoc
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6194
Build 8544: tox-on-jenkins
Build 8543: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ardumont added a comment.Jun 5 2019, 3:59 PM

Build has FAILED

You need to rebase your change on master

I have one diff (gnu lister) already rebased on the master, how can I have both the diffs rebased on master?

Use git branches (rebased on latest master) and arc diff from those.

nahimilega updated this revision to Diff 5099.Jun 5 2019, 4:05 PM

Added an important note in lister tutorial

swh-public-ci added a comment.Jun 5 2019, 4:08 PM

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

Harbormaster completed remote builds in B6046: Diff 5099.Jun 5 2019, 4:08 PM
ardumont mentioned this in D1482: GNU Lister.Jun 5 2019, 4:22 PM
nahimilega edited the summary of this revision. (Show Details)Jun 5 2019, 5:16 PM
vlorentz accepted this revision.Jun 6 2019, 5:06 PM
This revision is now accepted and ready to land.Jun 6 2019, 5:06 PM
vlorentz resigned from this revision.Jun 12 2019, 1:56 PM
This revision now requires review to proceed.Jun 12 2019, 1:56 PM
nahimilega updated this revision to Diff 5211.Jun 13 2019, 12:18 PM
nahimilega edited the summary of this revision. (Show Details)

Add testing and how to run a new lister in docker section in tutorial, also fixed few grammatical errors

Harbormaster failed remote builds in B6183: Diff 5211!Jun 13 2019, 12:18 PM
ardumont requested changes to this revision.Jun 13 2019, 2:02 PM
ardumont added inline comments.
docs/tutorial.rst
17

I'd keep the original which i found clearer.

32

Please keep backward here, it's correct.

180

swh-lister

193

I'm wondering if we want to enter into so much details.

@douardda What do you think? Isn't docker-compose up enough?

223

Truncate the output.
Also, keep it formatted as the real cli do, it's nicer to read with indentation.

226

is creating new loading task not yet registered, you need to register that task type as well.

239

Well, yes and no.
We will try to avoid using python top-level, let's use the scheduler instead.

You need to schedule a task, for example with the gnu lister:

swh scheduler --url http://localhost:5008/ task add list-gnu-full --policy oneshot
243

It's missing a new chapter that explicits we are changing subject here.

Ok, i think i see now.
Keep the original chapter as it is already.

Then add a new one to explicit how to run (and test) lister within the docker-dev environment.
After this one...

Let's discuss this in irc...

This revision now requires changes to proceed.Jun 13 2019, 2:02 PM
nahimilega marked an inline comment as done.Jun 13 2019, 2:12 PM
nahimilega added inline comments.
docs/tutorial.rst
193

Here I intentionally did this because docker-compose up will start all the containers with could be harsh on the pc. Running all the docker containers while working eats up all my RAM

nahimilega updated this revision to Diff 5221.Jun 13 2019, 2:39 PM
nahimilega marked 6 inline comments as done.

Made suggested changes

Harbormaster failed remote builds in B6194: Diff 5221!Jun 13 2019, 2:40 PM
nahimilega updated this revision to Diff 5224.Jun 13 2019, 3:22 PM

Make recommended changes

swh-public-ci added a comment.Jun 13 2019, 3:24 PM

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

Harbormaster completed remote builds in B6197: Diff 5224.Jun 13 2019, 3:25 PM
ardumont retitled this revision from Added an important note in lister tutorial. to tutorial: How to run a new lister (within docker-dev).Jun 13 2019, 7:34 PM
nahimilega updated this revision to Diff 5251.Jun 14 2019, 4:52 PM
  • Add new page "run_a new_lister"
swh-public-ci added a comment.Jun 14 2019, 4:56 PM

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

Harbormaster completed remote builds in B6224: Diff 5251.Jun 14 2019, 4:56 PM
ardumont requested changes to this revision.Jun 14 2019, 6:43 PM
ardumont added inline comments.
docs/run_a_new_lister.rst
4 ↗(On Diff #5251)

rename this to .. _run-lister-tutorial

You should then be able to link it

9 ↗(On Diff #5251)

docker-dev. It provides an almost production-like environment.

11 ↗(On Diff #5251)

when running in production.

I think we can drop Hence ... deployment.

16 ↗(On Diff #5251)
  1. Edit a docker-compose.override.yml, following the sample provided:
29 ↗(On Diff #5251)

Highlight Preparation steps and Configuration

63 ↗(On Diff #5251)

. You (missing a space in between)

64 ↗(On Diff #5251)

scheduler

docs/tutorial.rst
170

You say it already in the page, just propose it:

After tests, it's suggested to run your new lister in docker-dev: `How to run a lister <Link_to_the_page>`.
193

Yes, but that's a detail of your machine (as annoying as it is).
There should be a global note (which i think already exists) in the main part of the docker-dev (or getting-started) which explains the caveats.

Also, by exposing the service names here, that pose a potential problem if we ever change it (docker-compose won't though).

This revision now requires changes to proceed.Jun 14 2019, 6:43 PM
nahimilega added inline comments.Jun 14 2019, 7:04 PM
docs/run_a_new_lister.rst
67–68 ↗(On Diff #5251)

I tested this method of running lister, but when I used it, it only created a task of "list-gnu-full", nothing else. No other code was executed, neither more task of "load-gnu" created.
Whereas when I used in python code which was mentioned in README, it ran pretty nice and created loading tasks.

Can you please tell me what could be the reason for this behaviour?

nahimilega updated this revision to Diff 5257.Jun 14 2019, 9:19 PM
nahimilega marked 8 inline comments as done.
  • Fixed the docunment according to suggestion
swh-public-ci added a comment.Jun 14 2019, 9:24 PM

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

Harbormaster completed remote builds in B6233: Diff 5257.Jun 14 2019, 9:24 PM
nahimilega marked 3 inline comments as done.Jun 16 2019, 10:40 AM
ardumont requested changes to this revision.Jun 17 2019, 2:39 PM
ardumont added inline comments.
docs/run_a_new_lister.rst
9 ↗(On Diff #5257)
... docker-dev. This provides ...

Prefer short sentence when possible.

12 ↗(On Diff #5257)
To run a lister within your local environment:
27 ↗(On Diff #5257)

Please explain that this use docker with mount points on your local environment (your modified code) and not the pypi one.
That allows to execute the development version.

39 ↗(On Diff #5257)

You are missing the backend name (fqdn of the lister task needed for the code to run) as 2nd parameter of the cli, please fix this to:

swh scheduler task-type add list-gnu-full "swh.lister.gnu.tasks.GNUListerTask" "Full GNU lister" \
  --default-interval '1 day' \
  --backoff-factor 1

Note: that must be the reason of your error below.

docs/tutorial.rst
166
When developing a new lister, it's important to test. 
For this, add the tests (check `swh/lister/*/tests/`) and
register the celery tasks in the main conftest.py (`swh/lister....`).

Another important step is to actually run it within the 
docker-dev (:ref:`run-lister-tutorial`).
This revision now requires changes to proceed.Jun 17 2019, 2:39 PM
nahimilega updated this revision to Diff 5292.Jun 18 2019, 10:37 AM
nahimilega marked 4 inline comments as done.
  • Made recommended changes
docs/run_a_new_lister.rst
39 ↗(On Diff #5257)

I added the task type as you recommended.

(swh) archit@work-pc:~/swh-environment/swh-lister$ swh scheduler task-type list -v
...
list-gnu-full: swh.lister.gnu.tasks.GNUListerTask
  Full GNU lister
  interval: 1 day, 0:00:00 [None, None]
  backoff_factor: 1.0
  max_queue_length: None
  num_retries: None
  retry_delay: None
...

Now I created the task as

swh scheduler --url http://localhost:5008/ task add \
      list-gnu-full --policy oneshot

But when I checked for tasks created by the lister, there were none

(swh) archit@work-pc:~/swh-environment/swh-lister$ swh scheduler task list
Found 3 tasks

Task 16809
  Next run: 12 minutes ago (2019-06-17 19:02:11+00:00)
  Interval: 1 day, 0:00:00
  Type: list-gnu-full
  Policy: oneshot
  Status: disabled
  Priority: 
  Args:
  Keyword args:

There is something still something wrong. The lister is not creating the loading task.
Can you please help me find where the steps went wrong.

swh-public-ci added a comment.Jun 18 2019, 10:42 AM

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

Harbormaster completed remote builds in B6271: Diff 5292.Jun 18 2019, 10:42 AM
ardumont added inline comments.Jun 18 2019, 11:44 AM
docs/run_a_new_lister.rst
39 ↗(On Diff #5257)

Have you added the associated load-gnu task (as correctly mentioned in the doc ;)?

ardumont added a comment.Jun 19 2019, 8:48 AM

I did not see it earlier, in your command sample, please insert a space between $ and the start of the command.
I think it's way clearer to read.
Also it eases copy/paste from a user perspective (one cannot always see clearly the limit between the prompt and the start of the cli she wants to execute).

ardumont requested changes to this revision.Jun 19 2019, 8:52 AM

Almost there (again).
But this time, i'm right (hopefully ;)

docs/run_a_new_lister.rst
73 ↗(On Diff #5292)

You can also add a note to check that the lister's cache (table in db) has been indeed filled.

docs/tutorial.rst
174

That paragraph is redundant with the previous one which is much better (which also has sphinx ready link).
Please remove it.

This revision now requires changes to proceed.Jun 19 2019, 8:52 AM
nahimilega updated this revision to Diff 5380.Jun 20 2019, 12:38 AM
nahimilega marked an inline comment as done.
  • Add point 3 in 'run a new lister' and made recommended changes
nahimilega marked 3 inline comments as done.Jun 20 2019, 12:39 AM
swh-public-ci added a comment.Jun 20 2019, 12:42 AM

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

Harbormaster completed remote builds in B6364: Diff 5380.Jun 20 2019, 12:42 AM
ardumont accepted this revision.Jun 20 2019, 10:00 AM
This revision is now accepted and ready to land.Jun 20 2019, 10:00 AM
nahimilega added a comment.Jun 20 2019, 10:37 AM

I am not able to push this

(swh) archit@work-pc:~/swh-environment/swh-lister$ git push
Counting objects: 9, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 3.58 KiB | 1.19 MiB/s, done.
Total 9 (delta 6), reused 0 (delta 0)
remote: *** PUSH REJECTED BY COMMIT HOOK ***
remote: 
remote: This push was rejected by Herald push rule H16.
remote:     Change: commit/
remote:       Rule: Non-developers require code review for pushing content
remote:     Reason: needs review
remote: Transcript: https://forge.softwareheritage.org/herald/transcript/146188/
remote: 
To forge.softwareheritage.org:/source/swh-lister.git
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'git@forge.softwareheritage.org:/source/swh-lister.git'
nahimilega updated this revision to Diff 5388.Jun 20 2019, 10:44 AM

Rebase on latest master

nahimilega added a comment.Jun 20 2019, 10:45 AM
In D1441#36876, @nahimilega wrote:

I am not able to push this

I think because I have rebased it on latest master after merging D1612. This could be the reason

This revision was landed with ongoing or failed builds.Jun 20 2019, 10:46 AM
Closed by commit rDLSb2c6ddc35b6b: tutorial: Add testing and how to run section (authored by nahimilega). · Explain Why
This revision was automatically updated to reflect the committed changes.
swh-public-ci added a comment.Jun 20 2019, 10:48 AM

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

Harbormaster completed remote builds in B6372: Diff 5388.Jun 20 2019, 10:48 AM

Revision Contents
Changeset List

PathSize
docs/
73 lines

Diff 5221

View Options

docs/tutorial.rst

Loading...
About · Developer information · Legal