search

globus / globus-compute

Globus Compute: High Performance Function Serving for Science
https://www.globus.org/compute
Apache License 2.0
148 stars 47 forks source link

get_batch_result() is not implemented #299

Closed ryanchard closed 3 years ago

ryanchard commented 4 years ago

We do not yet implement the get_batch_result() functionality on the SDK. Instead, we have get_batch_status. This is now out of date as the get_batch_status function returns the results. We should resolve this inconsistency.

pratik-99 commented 3 years ago

can you please explain me how do I solve this issue I'm new to contributions

ryanchard commented 3 years ago

Hi @pratik-99 ! We would really appreciate your contribution!

This issue is mainly a mistake with naming conventions. We have a get_batch_status function that returns results and a get_batch_result function that is not implemented. To fix this inconsistency I suggest we rename get_batch_status to get_batch_result, remove the existing get_batch_result method, and then update any references to the old get_batch_status function. I believe it is used in a couple of tests (e.g., here: https://github.com/funcx-faas/funcX/blob/dev/funcx_endpoint/funcx_endpoint/tests/test_batch_submit.py#L30 ) and also in the tutorial notebook (https://github.com/funcx-faas/examples/blob/main/notebooks/Introduction.ipynb ).

The best way to contribute is to make a fork of this repository and then create a pull request with the change. Let me know if you have any questions.

pratik-99 commented 3 years ago

Thank you sir for such a detailed explanation I made the changes to all the get_batch_status() functions and also made the pull request but it shows some error in lint with Flake E303 too many blank lines

pratik-99 commented 3 years ago

Please help me out with this I will again make any changes if required and make pull request

ryanchard commented 3 years ago

Thanks very much!

pratik-99 commented 3 years ago

Welcome :)