Page MenuHomeSoftware Heritage

Add url validation
ClosedPublic

Authored by moranegg on Jan 9 2018, 2:15 PM.

Details

Summary

Cheking compatibility between client url and the deposit's metadata entry url.
This will enable the usage of url for origin creation.

Resolves T868

Diff Detail

Repository
rDDEP Push deposit
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont requested changes to this revision.Jan 9 2018, 2:43 PM
ardumont added inline comments.
swh/deposit/api/private/deposit_check.py
82

We don't want to expose the implementation detail of how we do the check in the documentation ;)
We want to expose the intent.

A better docstring than my previous attempt would be:

Given a deposit, check its associated metadata for mandatory field presence.
   ...
101

I forgot (dict) instead of (), can you please add it?

119

Please, see comments line 143 below

125

Simply returning the result here would be fine.

Also a docstring would be good.

As a note, there is a typo in the variable's name ;)

142

I don't like setting variable outside of the __init__ construct, especially when we can avoid it :)
(I know we did it in other modules but for other reasons).

If we want to manipulate the metadata multiple times (as introduced here), we should have a method for it. Then the multiple check methods will manipulate those metadata. The intents are clearer that way.

So here, that would give:

  • _check_metadata stays as before (it only checks the metadata for field presence)
  • _check_deposit_metadata becomes just a metadata_get (it's in charge of aggregating metadata)
  • _check_url is almost like you propose but it's in charge of filtering the url fields from the metadata passed as parameter prior to the current proposed check.

Then no need for that intermediary variable.

This revision now requires changes to proceed.Jan 9 2018, 2:43 PM
  • Create origin from given url in metadata
  • Amend checks with common function _get_metadata
  • Amend checks with common function _get_metadata
  • Amend checks with common function _get_metadata

A nitpick on mismatched docstrings.

swh/deposit/api/private/deposit_check.py
82

Well, the docstring no longer matches... :)

98

That would be the right location for the previous docstring.

  • Amend checks with common function _get_metadata
This revision is now accepted and ready to land.Jan 9 2018, 4:21 PM
This revision was automatically updated to reflect the committed changes.