Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "//" in URL concatenation #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix "//" in URL concatenation #18

wants to merge 1 commit into from

Conversation

chris-hld
Copy link
Member

When the URL of the specified database ends with an "/", the concatenation results in "//".

@fietew
Copy link
Member

fietew commented Jan 10, 2017

Does this result in failed downloads? If yes, which?

@hagenw
Copy link
Member

hagenw commented Jan 10, 2017

I think @chris-hld reported it for example for https://github.com/TWOEARS/examples/tree/master/qoe_coloration

@fietew
Copy link
Member

fietew commented Jan 10, 2017

In principle, the additional slashed should not result in broken URL's. Compare here with here. Maybe this is related to another problem.

@fietew
Copy link
Member

fietew commented Jan 10, 2017

If I execute https://github.com/TWOEARS/examples/blob/master/qoe_coloration/colorationWfsCircularCenter.m I get the error:

Undefined variable "xml" or class "xml.dbGetFile".

Error in readHumanLabels (line 32)
labelFile = xml.dbGetFile(labelFile);

Error in estimateColoration (line 30)
    humanLabels = readHumanLabels(humanLabelFiles{ii});

Error in colorationWfsCircularCenter (line 34)
prediction = estimateColoration(sim, sourceFiles, sourceTypes, humanLabelFiles);

indicating that the code is still using the old database API. After fixing this, I get warnings like this:

Warning: Download failed
(url=https://dev.qu.tu-berlin.de/projects/twoears-getdata/repository/raw//experiments/2015-10-01_wfs_coloration/human_label_coloration_wfs_circular_center_noise.csv),
trying alternative database... 
> In db.downloadFile (line 43)
  In db.getFile (line 85)
  In readHumanLabels (line 32)
  In estimateColoration (line 30)
  In colorationWfsCircularCenter (line 34) 

We have set the new database (twoears-getdata) as the default database. The files, which are requested by the script do not exist anymore. However, as the API tries the alternative (old) database the download still works. The database documentation also states that the files for this coloration test have been renamed.

@hagenw
Copy link
Member

hagenw commented Jan 10, 2017

Ah, this makes sense. I will fix this and I guess this pull request is then not really needed.
@chris-hld: or can you provide another example where the download failed from the new database, where the data should be there?

@chris-hld
Copy link
Member Author

You are right, I was able to break it down to other issues in the end. I think the double slash is still not ideal, that's why I kept this PR open. It's somehow misleading while debugging. We could also just remove the last slash in the specified databases, since it is added anyhow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants