richard-better / pushbullet.py

A python client for http://pushbullet.com
MIT License
575 stars 110 forks source link

get_channel method doesn't support multiple pages of results #127

Open aaknitt opened 6 years ago

aaknitt commented 6 years ago

The get_channel method loads channels for the current user and then returns an error "No channel found with channel_tag" when the channel is not found. However it doesn't handle multiple pages of results which means this will break if you have more than 20 channels.

The code does have some methods that handle multiple pages of results, but it looks like it only is used for getting pushes, not channels.

aaknitt commented 6 years ago

Corrected this issue by modifying

get_data as follows:

    def _get_data(self, url, prms=None):
        resp = self._session.get(url,params=prms)

        if resp.status_code in (401, 403):
            raise InvalidKeyError()
        elif resp.status_code == 429:
            raise PushbulletError("Too Many Requests, you have been ratelimited")
        elif resp.status_code != requests.codes.ok:
            raise PushbulletError(resp.status_code)

and _load_channels as follows:

def _load_channels(self):
        self.channels = []
        get_more_channels = True
        resp_dict = self._get_data(self.CHANNELS_URL)
        while get_more_channels == True:
            print resp_dict
            channel_list = resp_dict.get("channels", [])
            for channel_info in channel_list:
                if channel_info.get("active"):
                    c = Channel(self, channel_info)
                    self.channels.append(c)
            if "cursor" in resp_dict:
                cursor = resp_dict.get("cursor")
                resp_dict = self._get_data(self.CHANNELS_URL,prms={'cursor' : cursor})
            else:
                get_more_channels = False

I'll see if I can figure out how to use git to submit a fix.

aaknitt commented 6 years ago

Completed pull request https://github.com/randomchars/pushbullet.py/pull/128

kovacsbalu commented 6 years ago

Can you please add some test for this functionality?

rbrcsk commented 3 years ago

Hey @aaknitt I'll merge your PR, (or something similar) and do a release this week.