Differential D410

config: remove trailing slashes from URLs
ClosedPublic
Actions

Authored by kparal on Jul 2 2015, 11:40 AM.

Details

Reviewers
tflink
mkrizek
jskladan
Maniphest Tasks
T520: double slashes in URLs don't work with local resultsdb frontend
Commits
rLTRN5b3472b3bd99: config: remove trailing slashes from URLs
Summary

This fixes double-slash issues in URLs served over werkzeug (used inside
Flask).

Test Plan

test suite works, I also ran rpmlint and upgradepath

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal retitled this revision from to config: remove trailing slashes from URLs.Jul 2 2015, 11:40 AM
kparal updated this object.
kparal edited the test plan for this revision. (Show Details)
kparal added reviewers: jskladan, tflink, mkrizek.
kparal added a task: T520: double slashes in URLs don't work with local resultsdb frontend.
tflink accepted this revision.Jul 7 2015, 3:32 AM

Overall, looks good to me. Listed some thoughts on the comments in the example yaml but I'm not terribly attached to the changes - either way is fine.

conf/taskotron.yaml.example
58–59

I'm not sure this has to be after every entry. I also think that changing the warning to something along the lines of "Trailing slashes at the end of URLs will lead to double slashes when rendered and not all application servers handle that gracefully" would be more precise than "don't do this!" written before every entry.

This revision is now accepted and ready to land.Jul 7 2015, 3:32 AM
kparal added inline comments.Jul 7 2015, 10:05 AM
conf/taskotron.yaml.example
58–59

I have added a longer explanation under the Resources section, so people read the whole config file, they will find it.

I'd rather keep the short note alongside every affected item, because it's very easy to overlook this and you don't always read the whole config file. And if you don't know about this issue, it might take you quite some time to figure out.

Closed by commit rLTRN5b3472b3bd99: config: remove trailing slashes from URLs (authored by kparal). · Explain WhyJul 7 2015, 10:25 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 1104

View Options

conf/taskotron.yaml.example

Show All 35 Lines
36#report_to_bodhi: True 36​#report_to_bodhi: True
37 37​
38## Whether to send test results to the configured ResultsDB server. See also 38​## Whether to send test results to the configured ResultsDB server. See also
39## 'reporting_enabled' option. 39​## 'reporting_enabled' option.
40#report_to_resultsdb: True 40​#report_to_resultsdb: True
41 41​
42 42​
43## ==== RESOURCES section ==== 43​## ==== RESOURCES section ====
44## This section specifies access details to various external services 44​## This section specifies access details to various external services.
45​##
46​## Note: Try to keep custom URL addresses without a trailing slash. Otherwise
47​## the rendered URLs might end up containing double slashes, which some
48​## application servers don't handle gracefully (e.g. Flask's internal app
49​## server werkzeug).
45 50​
46## URL of Koji instance used for querying about new builds 51​## URL of Koji instance used for querying about new builds
47#koji_url: http://koji.fedoraproject.org/kojihub 52​#koji_url: http://koji.fedoraproject.org/kojihub
48 53​
49## URL of repository of all the RPM packages built in Koji 54​## URL of repository of all the RPM packages built in Koji
50#pkg_url: http://kojipkgs.fedoraproject.org/packages 55​#pkg_url: http://kojipkgs.fedoraproject.org/packages
51 56​
52## URL of Bodhi instance used for communication about package updates 57​## URL of Bodhi instance used for communication about package updates
53#bodhi_server: https://admin.fedoraproject.org/updates/ 58​## Please make sure the URL doesn't have a trailing slash.
59​#bodhi_server: https://admin.fedoraproject.org/updates
tflinkUnsubmitted
Not Done

I'm not sure this has to be after every entry. I also think that changing the warning to something along the lines of "Trailing slashes at the end of URLs will lead to double slashes when rendered and not all application servers handle that gracefully" would be more precise than "don't do this!" written before every entry.

kparalAuthorUnsubmitted
Not Done

I have added a longer explanation under the Resources section, so people read the whole config file, they will find it.

I'd rather keep the short note alongside every affected item, because it's very easy to overlook this and you don't always read the whole config file. And if you don't know about this issue, it might take you quite some time to figure out.

54 60​
55## Number of maximum retries to Bodhi server if the query request 61​## Number of maximum retries to Bodhi server if the query request
56## fails with 5xx HTTP error 62​## fails with 5xx HTTP error
57#bodhi_request_max_retries: 3 63​#bodhi_request_max_retries: 3
58 64​
59## URL of ResultsDB server API interface, which can store all test results 65​## URL of ResultsDB server API interface, which can store all test results.
60#resultsdb_server: http://127.0.0.1/resultsdb_api/api/v1.0/ 66​## Please make sure the URL doesn't have a trailing slash.
61 67​#resultsdb_server: http://127.0.0.1/resultsdb_api/api/v1.0
62## URL of ResultsDB frontend, which displays results from ResultsDB 68​
63#resultsdb_frontend: http://127.0.0.1/resultsdb/ 69​## URL of ResultsDB frontend, which displays results from ResultsDB.
64 70​## Please make sure the URL doesn't have a trailing slash.
65## URL of ExecDB server API interface, which tracks task execution status 71​#resultsdb_frontend: http://127.0.0.1/resultsdb
66#execdb_server: http://127.0.0.1/execdb/ 72​
67 73​## URL of ExecDB server API interface, which tracks task execution status.
68## URL of taskotron buildmaster 74​## Please make sure the URL doesn't have a trailing slash.
69#taskotron_master: http://127.0.0.1/taskmaster/ 75​#execdb_server: http://127.0.0.1/execdb
70 76​
71## URL of artifacts base directory 77​## URL of taskotron buildmaster, to construct log URLs from.
72#artifacts_baseurl: http://127.0.0.1/artifacts/ 78​## Please make sure the URL doesn't have a trailing slash.
79​#taskotron_master: http://127.0.0.1/taskmaster
80​
81​## URL of artifacts base directory, to construct artifacts URLs from.
82​## Please make sure the URL doesn't have a trailing slash.
83​#artifacts_baseurl: http://127.0.0.1/artifacts
73 84​
74## name of step in buildbot that executes the task 85​## name of step in buildbot that executes the task
75#buildbot_task_step = 'runtask' 86​#buildbot_task_step = 'runtask'
76 87​
77## Whether to cache downloaded files to speed up subsequent downloads. If True, 88​## Whether to cache downloaded files to speed up subsequent downloads. If True,
78## files will be downloaded to a common directory specified by "cachedir". At 89​## files will be downloaded to a common directory specified by "cachedir". At
79## the moment, Taskotron only supports Koji RPM downloads to be cached. 90​## the moment, Taskotron only supports Koji RPM downloads to be cached.
80## [default: False for production, True for development] 91​## [default: False for production, True for development]
▲ Show 20 Lines • Show All 45 Lines • ▼ Show 20 Line(s)
126## ==== SECRETS section ==== 137​## ==== SECRETS section ====
127## All login credentials and other secrets are here. If you add some secret 138​## All login credentials and other secrets are here. If you add some secret
128## here, make sure you make this file readable just for the right user accounts. 139​## here, make sure you make this file readable just for the right user accounts.
129 140​
130## FAS (Fedora Accounts System) credentials 141​## FAS (Fedora Accounts System) credentials
131## These credentials are used when reporting results into Bodhi. 142​## These credentials are used when reporting results into Bodhi.
132#fas_username: taskotron 143​#fas_username: taskotron
133#fas_password: '' 144​#fas_password: ''
134 ​
View Options

libtaskotron/config_defaults.py

Show First 20 Lines • Show All 46 Lines • ▼ Show 20 Line(s)23class Config(object):
47 profile = ProfileName.DEVELOPMENT #: 47​ profile = ProfileName.DEVELOPMENT #:
48 48​
49 reporting_enabled = False #: 49​ reporting_enabled = False #:
50 report_to_bodhi = True #: 50​ report_to_bodhi = True #:
51 report_to_resultsdb = True #: 51​ report_to_resultsdb = True #:
52 52​
53 koji_url = 'http://koji.fedoraproject.org/kojihub' #: 53​ koji_url = 'http://koji.fedoraproject.org/kojihub' #:
54 pkg_url = 'http://kojipkgs.fedoraproject.org/packages' #: 54​ pkg_url = 'http://kojipkgs.fedoraproject.org/packages' #:
55 bodhi_server = 'https://admin.fedoraproject.org/updates/' #: 55​ bodhi_server = 'https://admin.fedoraproject.org/updates' #:
56 execdb_server = 'http://127.0.0.1/execdb/' #: 56​ execdb_server = 'http://127.0.0.1/execdb' #:
57 resultsdb_server = 'http://127.0.0.1/resultsdb_api/api/v1.0/' #: 57​ resultsdb_server = 'http://127.0.0.1/resultsdb_api/api/v1.0' #:
58 resultsdb_frontend = 'http://127.0.0.1/resultsdb/' #: 58​ resultsdb_frontend = 'http://127.0.0.1/resultsdb' #:
59 taskotron_master = 'http://127.0.0.1/taskmaster/' #: 59​ taskotron_master = 'http://127.0.0.1/taskmaster' #:
60 artifacts_baseurl = 'http://127.0.0.1/artifacts/' #: 60​ artifacts_baseurl = 'http://127.0.0.1/artifacts' #:
61 buildbot_task_step = 'runtask' #: 61​ buildbot_task_step = 'runtask' #:
62 download_cache_enabled = True #: 62​ download_cache_enabled = True #:
63 63​
64 bodhi_posting_comments_span = 4320 #: 64​ bodhi_posting_comments_span = 4320 #:
65 # 3 days (3*24*60 = 4320) 65​ # 3 days (3*24*60 = 4320)
66 bodhi_request_max_retries = 3 #: 66​ bodhi_request_max_retries = 3 #:
67 67​
68 tmpdir = '/var/tmp/taskotron' #: 68​ tmpdir = '/var/tmp/taskotron' #:
▲ Show 20 Lines • Show All 49 Lines • Show Last 20 Lines