jrxFive / python-nomad

Client library Hashicorp Nomad
https://python-nomad.readthedocs.io/en/latest/
MIT License
139 stars 73 forks source link

added support for namespace in get_jobs #115

Closed fumblehool closed 3 years ago

jonathanrcross commented 3 years ago

Hi @fumblehool, Thanks for putting this PR. I was curious if we should start adding **kwargs to each method since the Nomad API is still changing and that having to keep adding and figuring out which parameters are compatible for which version may be an impossible task. So in this case it would look a bit like this

   def get_jobs(self, prefix=None, **kwargs):
        """ Lists all the jobs registered with Nomad.
           https://www.nomadproject.io/docs/http/jobs.html
            arguments:
              - prefix :(str) optional, specifies a string to filter jobs on based on an prefix.
                        This is specified as a querystring parameter.
              **kwargs: (dict), of querystring parameters to be sent.
            returns: list
            raises:
              - nomad.api.exceptions.BaseNomadException
              - nomad.api.exceptions.URLNotFoundNomadException
        """
get_jobs(self, prefix=None, namespace="some-namespace")

Merging dicts in python 2.7 has a couple more steps than python 3.5+, we'll have a helper utility function that we reference, instead of doing the below in each method.

params = {"prefix": ""}
kwargs = {"namespace": "some-namespace"}

merged_params = params.copy()
merged_params.update(kwargs)
merged_params
{'prefix': '', 'namespace': 'some-namespace'}

return self.request(method="get", params=merged_params).json()
resmo commented 3 years ago

@jonathanrcross I would like to merge it as is. Any objections?

jonathanrcross commented 3 years ago

Yup, all good to merge! Thanks @fumblehool! I'll follow up with a PR with what was described.