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

In the jsoc client, for the fetch function, max_conn is listed as a parameter in the documentation but its not a valid parameter. #3844

Closed
Raahul-Singh opened this issue Feb 24, 2020 · 4 comments · Fixed by #3822
Labels
Documentation Affects the documentation

Comments

@Raahul-Singh
Copy link
Contributor

Description

The title is pretty self-explanatory. This is at line 397 in file ~sunpy/net/jsoc/jsoc.py
The function snippet below contains the bug. Either max_conn be removed from the docstring or should be implemented.

    def fetch(self, jsoc_response, path=None, progress=True, overwrite=False,
              downloader=None, wait=True, sleep=10):
        """
        Make the request for the data in a JSOC response and wait for it to be
        staged and then download the data.

        Parameters
        ----------
        jsoc_response : `~sunpy.net.jsoc.jsoc.JSOCResponse` object
            A response object

        path : `str`
            Path to save data to, defaults to SunPy download dir

        progress : `bool`, optional
            If `True` show a progress bar showing how many of the total files
            have been downloaded. If `False`, no progress bar will be shown.

        overwrite : `bool` or `str`, optional
            Determine how to handle downloading if a file already exists with the
            same name. If `False` the file download will be skipped and the path
            returned to the existing file, if `True` the file will be downloaded
            and the existing file will be overwritten, if `'unique'` the filename
            will be modified to be unique.

        max_conns : `int`
            Maximum number of download connections.

        downloader : `parfive.Downloader`, optional
            The download manager to use.

        wait : `bool`, optional
           If `False` ``downloader.download()`` will not be called. Only has
           any effect if `downloader` is not `None`.

        sleep : `int`
            The number of seconds to wait between calls to JSOC to check the status
            of the request.

        Returns
        -------
        results : a `~sunpy.net.download.Results` instance
            A Results object

        """
        # Make staging request to JSOC
        responses = self.request_data(jsoc_response)

        # Make response iterable
        if not isiterable(responses):
            responses = [responses]

        # Add them to the response for good measure
        jsoc_response.requests = [r for r in responses]
        time.sleep(sleep/2.)

        for response in responses:
            response.wait(verbose=progress)

        return self.get_request(responses, path=path, overwrite=overwrite,
                                progress=progress, downloader=downloader,
                                wait=wait)

@Cadair
Copy link
Member

Cadair commented Feb 24, 2020

It should be removed from the docstring. Although I thought other clients passed through kwargs to parfive.enqueue_file, which includes max_conn?

@Raahul-Singh
Copy link
Contributor Author

Raahul-Singh commented Feb 24, 2020

It should be removed from the docstring. Although I thought other clients passed through kwargs to parfive.enqueue_file, which includes max_conn?

HelioViever (helioviever.py line 418) doesn't seem to do so.

    def _get_file(self, params, progress=True, directory=None, overwrite=False):
        """Downloads a file and return the filepath to that file."""
        if directory is None:
            directory = Path(sunpy.config.get('downloads', 'download_dir'))
        else:
            directory = Path(directory).expanduser().absolute()

        downloader = parfive.Downloader(progress=progress, overwrite=overwrite)

        url = urllib.parse.urljoin(self._api,
                                   "?" + urllib.parse.urlencode(params))
        downloader.enqueue_file(url, path=directory)

        res = downloader.download()

        if len(res) == 1:
            return res[0]
        else:
            return res

@0xamogh
Copy link
Contributor

0xamogh commented Feb 25, 2020

Can I take this up?

@Cadair
Copy link
Member

Cadair commented Feb 25, 2020

I think it's likely to get addressed as part of #3822

@dstansby dstansby added the Documentation Affects the documentation label Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Affects the documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants