Summary

Fix SVN auth/HTTPS support with Subvertpy.

Review Request #10037 — Created June 24, 2018 and submitted June 29, 2018, 4:13 p.m.

Information

Owner

easyb

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

acd80cd...

Reviewers

Groups

reviewboard

People

Description

When using Subvertpy as an SCM backend, adding an HTTPS repository
requiring Basic authentication would fail showing:
A repository was not found at the specified path.

Weirdly, when passing the `subvertpy.AUTH_PARAM_DEFAULT_*` constants to
`subvertpy.ra.Auth.auth.set_parameter()` instead of the corresponding
strings, it works.
Also, in `accept_ssl_certificate()` username/password were not passed.

Testing Done


Created Docker container form easybe/alpine-reviewboard:wip
Sucessfully added an SVN repository requiring HTTPS and Basic auth

Issues

Description	From	Last Updated
Can you add some whitespace to break this up a little? We generally include blank lines around blocks. Something like …	david	June 26, 2018, 10:56 p.m.

flake8 passed.

JSHint passed.

Testing Done:

~		Sucessfully added an SVN repository requiring HTTPS and Basic auth.
	~	Created Docker container form easybe/alpine-reviewboard:wip
	+	Sucessfully added an SVN repository requiring HTTPS and Basic auth.

Testing Done:

		Created Docker container form easybe/alpine-reviewboard:wip
~		Sucessfully added an SVN repository requiring HTTPS and Basic auth.
	~	Sucessfully added an SVN repository requiring HTTPS and Basic auth

Description:

		When using Subvertpy as an SCM backend, adding an HTTPS repository
		requiring Basic authentication would fail showing:
		A repository was not found at the specified path.

		Weirdly, when passing the `subvertpy.AUTH_PARAM_DEFAULT_*` constants to
		`subvertpy.ra.Auth.auth.set_parameter()` instead of the corresponding
		strings, it works.
~		Also, in `accept_ssl_certificate()` username/password where not passed.
	~	Also, in `accept_ssl_certificate()` username/password were not passed.

Summary:

-	Fix SVN auth/SSL support with Subvertpy.
+	Fix SVN auth/HTTPS support with Subvertpy.

Thanks so much for this fix! Just one formatting request.

reviewboard/scmtools/svn/subvertpy.py (Diff revision 1)

The issue has been resolved. Show all issues

Can you add some whitespace to break this up a little? We generally include blank lines around blocks. Something like this:

...

username = self.auth.get_parameter(AUTH_PARAM_DEFAULT_USERNAME)

if username:
    auth.set_parameter(AUTH_PARAM_DEFAULT_USERNAME, B(username))

password = self.auth.get_parameter(AUTH_PARAM_DEFAULT_PASSWORD)

if password:
    auth.set_parameter(AUTH_PARAM_DEFAULT_PASSWORD, B(password))

...