Details
- Reviewers
ardumont anlambert - Group Reviewers
Reviewers - Commits
- rDWAPPS6722da987bd4: add support for the CVS loader to 'Save Code Now'
Diff Detail
- Repository
- rDWAPPS Web applications
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/965/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/965/console
Build has FAILED
Patch application report for D5992 (id=21628)
Rebasing onto 4a9cf1d618...
Current branch diff-target is up to date.
Changes applied before test
commit 19dca75c0ff7f215fe9bc74b9c74e86873fef75d Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now'
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/966/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/966/console
Build has FAILED
Patch application report for D5992 (id=21791)
Rebasing onto 481647f863...
Current branch diff-target is up to date.
Changes applied before test
commit 953d420949a8c49bccc4e878e593ca8a83086ce3 Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now'
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/984/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/984/console
- add support for the CVS loader to 'Save Code Now'
- sort list of visit types alphabetically
Build is green
Patch application report for D5992 (id=22241)
Rebasing onto 66865c37ce...
Current branch diff-target is up to date.
Changes applied before test
commit 253bf73c0e657938961dac71124005ffde17728a Author: Stefan Sperling <stsp@stsp.name> Date: Thu Aug 26 14:02:17 2021 +0200 sort list of visit types alphabetically commit e25e91d9b101378fbd07f681285548e946384f9e Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now'
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1036/ for more details.
Also I just realized, as it's not completely ready afaik, don't land it yet though.
@anlambert What do you think of this?
Do we want for example to have a configuration (file) option to hide it and we toggle it later, when it's ready?
That'd avoid to let this diff hang.
TIA
Instead of relying on a configuration option, we could check if the load-cvs task type is present in the scheduler database
and activate the save code now for cvs based on that result.
Currently it is not, see below:
print( [ t["type"] for t in swhweb_config["scheduler"].get_task_types() if "load" in t["type"] ] )
['load-hg', 'load-svn', 'load-npm', 'load-pypi', 'load-git', 'load-svn-from-archive', 'load-hg-from-archive', 'load-git-from-dir', 'load-git-from-zip', 'load-nixguix', 'load-deb-package', 'load-deposit', 'load-archive-files', 'load-svn-from-remote-dump', 'load-cran']
What do you think ?
What do you think ?
yes, good idea. That'd simplify the setup both in docker and prod (as in nothing to do ;)
@stsp , thanks for this !
I am putting that diff in changes requested mode as I made some changes in swh-web (D6170, D6184)
to allow the addition of a new visit type to 'Save code now' even if the associated loader is not yet available
in production.
It will allow you to test save code now for cvs in docker environment while ensuring the cvs visit type will
be available in production only when its loader deployed.
Please find below your diff adapted once rebased on latest swh-web HEAD.
diff --git a/assets/src/bundles/save/index.js b/assets/src/bundles/save/index.js index d2c54ac7..1a893b32 100644 --- a/assets/src/bundles/save/index.js +++ b/assets/src/bundles/save/index.js @@ -358,7 +358,7 @@ export function validateSaveOriginUrl(input) { } if (validUrl) { - const allowedProtocols = ['http:', 'https:', 'svn:', 'git:']; + const allowedProtocols = ['http:', 'https:', 'svn:', 'git:', 'rsync:', 'pserver:', 'ssh:']; validUrl = ( allowedProtocols.find(protocol => protocol === originUrl.protocol) !== undefined ); diff --git a/cypress/integration/origin-save.spec.js b/cypress/integration/origin-save.spec.js index 0c245eb0..47dc1c84 100644 --- a/cypress/integration/origin-save.spec.js +++ b/cypress/integration/origin-save.spec.js @@ -19,8 +19,8 @@ const saveCodeMsg = { 'csrfError': 'CSRF Failed: Referrer checking failed - no Referrer.' }; -const anonymousVisitTypes = ['git', 'hg', 'svn']; -const allVisitTypes = ['archives', 'git', 'hg', 'svn']; +const anonymousVisitTypes = ['cvs', 'git', 'hg', 'svn']; +const allVisitTypes = ['archives', 'cvs', 'git', 'hg', 'svn']; function makeOriginSaveRequest(originType, originUrl) { cy.get('#swh-input-origin-url') diff --git a/swh/web/common/origin_save.py b/swh/web/common/origin_save.py index d088cb26..62d74e81 100644 --- a/swh/web/common/origin_save.py +++ b/swh/web/common/origin_save.py @@ -121,7 +121,12 @@ def can_save_origin(origin_url: str, bypass_pending_review: bool = False) -> str # map visit type to scheduler task # TODO: do not hardcode the task name here (T1157) -_visit_type_task = {"git": "load-git", "hg": "load-hg", "svn": "load-svn"} +_visit_type_task = { + "git": "load-git", + "hg": "load-hg", + "svn": "load-svn", + "cvs": "load-cvs", +} _visit_type_task_privileged = { "archives": "load-archive-files", @@ -203,7 +208,9 @@ def _check_visit_type_savable(visit_type: str, privileged_user: bool = False) -> ) -_validate_url = URLValidator(schemes=["http", "https", "svn", "git"]) +_validate_url = URLValidator( + schemes=["http", "https", "svn", "git", "rsync", "pserver", "ssh"] +) def _check_origin_url_valid(origin_url: str) -> None: diff --git a/swh/web/config.py b/swh/web/config.py index dba7378f..a78c7434 100644 --- a/swh/web/config.py +++ b/swh/web/config.py @@ -34,6 +34,7 @@ ORIGIN_VISIT_TYPES = [ "pypi", "svn", "tar", + "cvs", ] diff --git a/swh/web/templates/misc/origin-save.html b/swh/web/templates/misc/origin-save.html index 60fa8d72..7d676721 100644 --- a/swh/web/templates/misc/origin-save.html +++ b/swh/web/templates/misc/origin-save.html @@ -85,6 +85,9 @@ See top-level LICENSE file for more information <li><code>git</code>, for origins using <a href="https://git-scm.com/">Git</a></li> <li><code>hg</code>, for origins using <a href="https://www.mercurial-scm.org/">Mercurial</a></li> <li><code>svn</code>, for origins using <a href="https://subversion.apache.org/">Subversion</a></li> + {% if "cvs" in visit_types %} + <li><code>cvs</code>, for origins using <a href="http://cvs.nongnu.org/">CVS</a></li> + {% endif %} </ul> </li> <li><b>Origin url:</b> the url of the remote repository for the software origin.<br/> diff --git a/swh/web/tests/conftest.py b/swh/web/tests/conftest.py index fead2e30..751d2476 100644 --- a/swh/web/tests/conftest.py +++ b/swh/web/tests/conftest.py @@ -457,6 +457,21 @@ def swh_scheduler(swh_scheduler): "retry_delay": timedelta(hours=2), } ) + # create load-cvs task type + swh_scheduler.create_task_type( + { + "type": "load-cvs", + "description": "Update a CVS repository", + "backend_name": "swh.loader.cvs.tasks.LoadCVS", + "default_interval": timedelta(days=64), + "min_interval": timedelta(hours=12), + "max_interval": timedelta(days=64), + "backoff_factor": 2, + "max_queue_length": None, + "num_retries": 7, + "retry_delay": timedelta(hours=2), + } + ) # add method to add load-archive-files task type during tests def add_load_archive_task_type(): diff --git a/swh/web/tests/misc/test_origin_save.py b/swh/web/tests/misc/test_origin_save.py index b357a138..bd9561d2 100644 --- a/swh/web/tests/misc/test_origin_save.py +++ b/swh/web/tests/misc/test_origin_save.py @@ -14,7 +14,7 @@ from swh.web.common.origin_save import SAVE_REQUEST_ACCEPTED, SAVE_TASK_SUCCEEDE from swh.web.common.utils import reverse from swh.web.tests.utils import check_http_get_response -VISIT_TYPES = ("git", "svn", "hg") +VISIT_TYPES = ("git", "svn", "hg", "cvs") PRIVILEGED_VISIT_TYPES = tuple(list(VISIT_TYPES) + ["archives"])
Build has FAILED
Patch application report for D5992 (id=22960)
Rebasing onto 9e51595ad7...
Current branch diff-target is up to date.
Changes applied before test
commit e66e206750f05250b2fbaac8fa5c6de0cabc5053 Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now'
Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1113/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1113/console
Build is green
Patch application report for D5992 (id=22981)
Rebasing onto 8aa625784d...
First, rewinding head to replay your work on top of it... Applying: add support for the CVS loader to 'Save Code Now'
Changes applied before test
commit cfcc003ef664ba4fb51f6b58b74de9e204cac394 Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now' Includes changes from anlambert suggested during code review.
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1114/ for more details.
Looks good to me, just a small indentation issue to fix before landing this. Thanks !
swh/web/templates/misc/origin-save.html | ||
---|---|---|
87–91 | indentation is not correct here, these should be: <li><code>svn</code>, for origins using <a href="https://subversion.apache.org/">Subversion</a></li> {% if "cvs" in visit_types %} <li><code>cvs</code>, for origins using <a href="http://cvs.nongnu.org/">CVS</a></li> {% endif %} | |
swh/web/tests/conftest.py | ||
449 | thanks ! |
swh/web/templates/misc/origin-save.html | ||
---|---|---|
87–91 | This was a tabs vs. spaces issue. Will fix it. |
Build is green
Patch application report for D5992 (id=22987)
Rebasing onto 8aa625784d...
First, rewinding head to replay your work on top of it... Applying: add support for the CVS loader to 'Save Code Now'
Changes applied before test
commit b25af5e89cf0f00bc1dc062bfd9cf98a2f9bc0c3 Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now' Includes changes from anlambert suggested during code review.
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1115/ for more details.
Build is green
Patch application report for D5992 (id=22989)
Rebasing onto 8aa625784d...
Current branch diff-target is up to date.
Changes applied before test
commit 6722da987bd4c1eb5124ee2d6eebcbeca1ca0fde Author: Stefan Sperling <stsp@stsp.name> Date: Tue Jul 13 15:41:18 2021 +0200 add support for the CVS loader to 'Save Code Now' Includes changes from anlambert suggested during code review.
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/1116/ for more details.