Bug fixes for search functionality

[NickolausDS]

A bug in CRUD for both Search Subjects and Entries caused parameters
to be lost before the request was sent, resulting in errors from
Globus Search.

[sirosen]

merge_params modifies a dict in place and returns None. You don't want to use x = merge_params(x, ...) -- the current usage is correct.

See the merge_params docstring and contents (which are quite small) for detail:

globus-sdk-python/globus_sdk/base.py

Lines 404 to 444 in 1672d3c

	def merge_params(base_params, **more_params):
	"""
	Merge additional keyword arguments into a base dictionary of keyword
	arguments. Only inserts additional kwargs which are not None.
	This way, we can accept a bunch of named kwargs, a collector of additional
	kwargs, and then put them together sensibly as arguments to another
	function (typically BaseClient.get() or a variant thereof).
	For example:
	>>> def ep_search(self, filter_scope=None, filter_fulltext=None, **params):
	>>> # Yes, this is a side-effecting function, it doesn't return a new
	>>> # dict because it's way simpler to update in place
	>>> merge_params(
	>>> params, filter_scope=filter_scope,
	>>> filter_fulltext=filter_fulltext)
	>>> return self.get('endpoint_search', params=params)
	this is a whole lot cleaner than the alternative form:
	>>> def ep_search(self, filter_scope=None, filter_fulltext=None, **params):
	>>> if filter_scope is not None:
	>>> params['filter_scope'] = filter_scope
	>>> if filter_fulltext is not None:
	>>> params['filter_scope'] = filter_scope
	>>> return self.get('endpoint_search', params=params)
	the second form exposes a couple of dangers that are obviated in the first
	regarding correctness, like the possibility of doing
	>>> if filter_scope:
	>>> params['filter_scope'] = filter_scope
	which is wrong (!) because filter_scope='' is a theoretically valid,
	real argument we want to pass.
	The first form will also prove shorter and easier to write for the most
	part.
	"""
	for param in more_params:
	if more_params[param] is not None:
	base_params[param] = more_params[param]

I'm going to close this out, since we don't want this patch, but please open an issue with the actual bug details so that we can dig into this.

[NickolausDS]

#275

[sirosen]

Oh, wow! Someone just pointed out to me that I got this totally wrong.
I thought you were changing good usage to bad, but you were changing bad usage to good!

[sirosen]

Yes, this looks correct, now that I take a closer look at it.
Sorry for the runaround.

There aren't any tests for this, so I'll go ahead and merge without waiting for the testsuite to run.

[NickolausDS]

Thanks!