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 ability to define scopes #53

Merged
merged 3 commits into from Apr 28, 2021
Merged

Fix ability to define scopes #53

merged 3 commits into from Apr 28, 2021

Conversation

danpalmer
Copy link
Contributor

Currently the library raises an HTTP 400 Bad Request when scopes are passed to GhDeviceAuth because we attempt to serialise a one-tuple containing an iterable, instead of the iterable itself.

This change fixes auth to maintain the iterable all the way through so that serialisation works and the API can understand the scopes we send it.

Note to reviewers: I've not seen a codebase like this before, it looks like all the code is auto-generated from the notebooks? Can I rely on the package codegen working for me here?

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@hamelsmu
Copy link
Member

If you run nbdev_test_nbs you will see that this change breaks things

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-443a8df8cf7f> in <module>
      1 from ghapi.all import GhApi
----> 2 api = GhApi()

~/github/ghapi/ghapi/core.py in __init__(self, owner, repo, token, debug, limit_cb, **kwargs)
     92         if owner: kwargs['owner'] = owner
     93         if repo:  kwargs['repo' ] = repo
---> 94         funcs_ = L(funcs).starmap(_GhVerb, client=self, kwargs=kwargs)
     95         self.func_dict = {f'{o.path}:{o.verb.upper()}':o for o in funcs_}
     96         self.groups = {k.replace('-','_'):_GhVerbGroup(k,v) for k,v in groupby(funcs_, 'tag').items()}

~/github/fastcore/fastcore/foundation.py in starmap(self, f, *args, **kwargs)
    173         return self.map(lambda o: o.get(k,default) if isinstance(o, dict) else nested_attr(o,k,default))
    174
--> 175     def starmap(self, f, *args, **kwargs): return self._new(itertools.starmap(partial(f,*args,**kwargs), self))
    176     def zip(self, cycled=False): return self._new((zip_cycle if cycled else zip)(*self))
    177     def zipwith(self, *rest, cycled=False): return self._new([self, *rest]).zip(cycled=cycled)

~/github/fastcore/fastcore/foundation.py in _new(self, items, *args, **kwargs)
    108     @property
    109     def _xtra(self): return None
--> 110     def _new(self, items, *args, **kwargs): return type(self)(items, *args, use_list=None, **kwargs)
    111     def __getitem__(self, idx): return self._get(idx) if is_indexer(idx) else L(self._get(idx), use_list=None)
    112     def copy(self): return self._new(self.items.copy())

~/github/fastcore/fastcore/foundation.py in __call__(cls, x, *args, **kwargs)
     95     def __call__(cls, x=None, *args, **kwargs):
     96         if not args and not kwargs and x is not None and isinstance(x,cls): return x
---> 97         return super().__call__(x, *args, **kwargs)
     98
     99 # Cell

~/github/fastcore/fastcore/foundation.py in __init__(self, items, use_list, match, *rest)
    103     def __init__(self, items=None, *rest, use_list=False, match=None):
    104         if (use_list is not None) or not is_array(items):
--> 105             items = listify(items, *rest, use_list=use_list, match=match)
    106         super().__init__(items)
    107

~/github/fastcore/fastcore/basics.py in listify(o, use_list, match, *rest)
     54     elif isinstance(o, list): res = o
     55     elif isinstance(o, str) or is_array(o): res = [o]
---> 56     elif is_iter(o): res = list(o)
     57     else: res = [o]
     58     if match is not None:

~/github/ghapi/ghapi/core.py in __init__(self, path, verb, oper, summary, url, params, data, preview, client, kwargs)
     43     __slots__ = 'path,verb,tag,name,summary,url,route_ps,params,data,preview,client,__doc__'.split(',')
     44     def __init__(self, path, verb, oper, summary, url, params, data, preview, client, kwargs):
---> 45         tag,name = oper.split('/')
     46         name = name.replace('-','_')
     47         path,_,_ = partial_format(path, **kwargs)

ValueError: too many values to unpack (expected 2)

@hamelsmu
Copy link
Member

oh yes this is not your issue this is something unrelated. My apologies

@hamelsmu hamelsmu merged commit 4795c3b into fastai:master Apr 28, 2021
@danpalmer danpalmer deleted the patch-1 branch May 3, 2021 16:48
@danpalmer
Copy link
Contributor Author

@hamelsmu hey, any chance of a PyPI release with this fix?

@hamelsmu
Copy link
Member

hamelsmu commented May 11, 2021

@danpalmer I've tagged @jph00 to cut a release, he will do it when he has the chance. In the meantime you can do

pip install -U git+https://github.com/fastai/ghapi.git

@danpalmer
Copy link
Contributor Author

@jph00 hey, any update on a release?

We can work around this with a git-based install, but it slows down the build and forces us to either pin to a commit and miss updates, or not pin and possible get unreleased changes.

@jph00
Copy link
Member

jph00 commented May 23, 2021

@danpalmer Many apologies for the delay - will get onto it today

@jph00 jph00 added the bug Something isn't working label May 24, 2021
@jph00
Copy link
Member

jph00 commented May 24, 2021

@danpalmer pushed a release now. cc @hamelsmu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants