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

Bug: url_join string handling and get_latest_installable_revision not updated with other changes #629

Closed
wants to merge 9 commits into from

Conversation

chambm
Copy link
Contributor

@chambm chambm commented Aug 20, 2015

Change url_join() to handle strings as pathspec: previously it would split a string into an array of characters and separate them all by '/' - good times.

Fix /api/repositories/get_latest_installable_revision to use the tool_shed_get() syntax required by recent changes to that function.

…as determining a file is XML, i.e. not very)

* Add original_name parameter to Binary.is_binary_sniffable() static function and subclass' sniff() functions
* Pass dataset.name to Binary.is_binary_sniffable() in upload.py
* Add MZ5 format (a mass spectrometry data format based on HDF5); sniffer currently uses file extension, because proper detection would require h5py (or some godawful code to navigate a HDF5 file without the HDF5 library); I left in some TODOs with the h5py calls which will work as soon as h5py is available
… about making original_filename have a default of "" in every case, but decided I'll make that change if community prefers it
…h just call sniff(filename) don't need to be changed; I like how they must be having a runtime error due to wrong parameters but it just results in the 'txt' type :) - this changes many more files because I did replace all in files for "def sniff(\s*self, filename\s*)" -> "def sniff( self, filename, original_name="" )"
…ork with recent changes to common_util.url_join

* Fix common_util.url_join to handle a string as pathspec, instead of treating it as an a/r/r/a/y/ /o/f/ /c/h/a/r/a/c/t/e/r/s
…into dev"

This reverts commit 93bbcc3, reversing
changes made to fb11d71.
url = common_util.url_join( tool_shed_url,
'api/repositories/get_ordered_installable_revisions%s' % params )
params = dict(name=name, owner=owner)
pathspec = 'api/repositories/get_ordered_installable_revisions'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that for coherence with #586 this line should be changed to:

        pathspec = ['api', 'repositories', 'get_ordered_installable_revisions']

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would be more consistent, but what was the reasoning behind that syntax? It seems counter-intuitive: the documentation above refers to the API path in string notation.

@nsoranzo
Copy link
Member

Thanks a lot @chambm, very good catch!

This branch seems to contain also some commits from #603, maybe it would be better to start from a clean dev branch and add only the commit(s) fixing the url_join() bug.

nsoranzo added a commit that referenced this pull request Aug 21, 2015
Cleaner commit history for #629 and array syntax for pathspec
@nsoranzo
Copy link
Member

Superseded by #632.

@nsoranzo nsoranzo closed this Aug 21, 2015
@chambm chambm deleted the bug/url_join-latest_revision branch December 3, 2015 00:22
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.

None yet

2 participants