I copied this class from loader-tar, hence I assume we also change this in the loader-tar class to make it more efficient.

I'll update the loader-tar with this.

The list of tarballs is stored as the json string format in the model and needs to be converted back to list of dictionary before using it to create a loading task

But why do we need the list of tarballs in the model?

I'd expect we compute it and pass it along to the loader.
But we do not need to keep it in the lister's model.

Please correct me if I am wrong somewhere -
To pass the list of tarballs to the loader we need to pass it in the loading task created by the.

To pass the list of tarballs in the loading task we need it so that it can be accessed in the task_dict() function.

To access the list of tarballs in the task_dict() we need to ensure it is present in the model.

Please correct me if I am wrong somewhere -
To pass the list of tarballs to the loader we need to pass it in the loading task created by the.

I'm missing the end of the sentence.

To pass the list of tarballs in the loading task we need it so that it can be accessed in the task_dict() function.
To access the list of tarballs in the task_dict() we need to ensure it is present in the model.

I'm not sure yet. I'd need to dig in the code to make sure.

All I know is that it should not be a requisite to store the computed data (needed for the loader) in the lister model.
So if the actual code requires it, it feels wrong to me.

Please correct me if I am wrong somewhere -
To pass the list of tarballs to the loader we need to pass it in the loading task created by the.

I'm missing the end of the sentence.

Sorry for that, it is "To pass the list of tarballs to the loader we need to pass it in the loading task created."

I'm not sure yet. I'd need to dig in the code to make sure.

All I know is that it should not be a requisite to store the computed data (needed for the loader) in the lister model.

I tried to do what I thought would work fine, and I am not able to come up with another way to send the tarballs of loader. Can you please look up suggest some better way.

One way to avoid including tarballs in model is to make a variable instance of class named tarballs (like LISTER_NAME or TREE_URL), which would countain all the tarballs of each package and can be accessed from task_dict() function
Hence will eliminate the need of adding tarballs in model.

I'd expect we compute it and pass it along to the loader.

I mean that through the scheduler (so that's what the task_dict method here is all about).

I did it this way because if I remove safely_issue_request() method then I have to override the whole run() method from SimpleLister class just to change one line(ie the top line where safely_issue_request() is called), unnecessary increase in code.

Shall I keep it the way it is now or override the whole run() method?

Ok then.
Let's keep that.

I don't like doing that, altering code for tests, but i acknowledge we need to for now.
We need to rework lister base code and tests anyhow, so we'll see then.

It would be great if loader-tar can be shifted to the core. That would reduce the code to less than half

In the end, i think it'd be best to use simply use request to download that file (which is already a module dependency).
And not duplicate that code.

There has been discussion to refactor the distribution/package loader and that code will probably disappear (so no point in pushing it to core).

As this lister is listing all the repos in gnu, mirror and old-gnu directory, there are some files in the listed repos that are not tarball and hence are not required to be loaded, hence I thought of creating a separate gnu loader which would remove useless files and then load.

So I wish to make a new lister then also I have to match this according to loader-tar?

As this lister is listing all the repos in gnu, mirror and old-gnu directory, there are some files in the listed repos that are not tarball and hence are not required to be loaded, hence I thought of creating a separate gnu loader which would remove useless files and then load.

Discussing with @anlambert (no task yet), we entertained the idea of having a dedicated gnu loader.
It would mostly behave the same way as the npm/pypi loader (extract metadata from a metadata file, ingest the tarballs and reference them in the snapshot, result of the loading visit for that origin).

So that'd go in the direction of a dedicated loader as you mentioned.

For the files that are missing or files that are not tarballs, the lister simply needs to not list them (thus not passing them along to the loader).

So I wish to make a new lister then also I have to match this according to loader-tar?

I have trouble understanding the sentence.
No need for a new lister.

Sorry, there was a typo in this sentence, it should be
So I wish to make a new loader then also I have to match this according to loader-tar?

Have a look at [1], this should be simplified to create a task whose arguments are a list of dict.
The origin becomes the root directory and the tarballs to ingest are the listed tarballs from that root directory.
In the end the difficulty of that lister becomes to determine which root directory are worth listing (i'd say elect the ones having tarballs).

Dict of key values:

• archive: url targetting a tarball from a directory to retrieve from the gnu archive (and ingest in swh archive)
• date: the modification time the archive got uploaded to the gnu.org mirror (which should be used somehow, probably as the revision date). Initially it was for the loading visit but no more (as we are aiming at loading a

Later, the tarball loader should be adapted (again according to [1]) to ingest those tarballs and make the appropriate snapshot (result of the one loading visit).

[1] T1389#31790

There is no datatype to store a list of dictionary in Postgres, hence either we make a separate table for this, or as it is a list of dictionary ,we can convert it into json string and store it in string datatype in database, and convert it back to list of dictionary when needed. I used the JSON method over other database as it is easy to write and understand.

So checking the base code of the simple lister class (ingest_data).
The tarball key should be present at the moment of the task_dict creation for the scheduler.
So i stand by my initial statement, we should not have to transform back and forth in json the tarball dict.
It should stay as is and be out of the lister model.

Yes, they are necessary, they are mentioned as abc.abstract attribute in the simple lister class

You should remove those decorators from the base class.
It's not used here and makes the code immediatly harder to read and will make it harder to maintain later.

You should then remove those unused definitions (that might also simplify the cran lister diff).

I created D1566 to address this issue

I took inspiration from PyPI lister, there it had underscore prefix hence I opted to follow the same style. Shall I change it?

You can drop the _, it was to be read as a private variable. It seems to be unneeded, so simply drop it ;)

please, rename to a simpler name.

That'd be simpler to read.

filename = single_file['name']
".tar" in filename or ".zip" in filename

If we do this, then it will also include files with extension tar.gz.sig. I don't think we need to add those files as they do not contain any code.

Indeed, that's inconvenient.

I guess gnu's manifest (tree.json.gz) is not moving a lot so the way forward would be to analyze and compulse the list of possible extension files from that manifest (take a snapshot of today would be enough).
While possibly tedious, it'd be the right way to determine the right implementation here.

Could you try and investigate in that direction?
(and update the task with such analysis result)

Thanks

In my view, the best method to check for tarball would be to break the filename on " . " and check if the word between last and second last " . " is "tar" or not?. If it is "tar" then the file is useful, else the file does not have source code.

No worries, that happens.

docstring update to:
"Keep only gnu and old-gnu folders from JSON."