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

A version control could be used across the interface #116

Closed
khsrali opened this issue Jul 30, 2024 · 2 comments · Fixed by #128
Closed

A version control could be used across the interface #116

khsrali opened this issue Jul 30, 2024 · 2 comments · Fixed by #128

Comments

@khsrali
Copy link
Contributor

khsrali commented Jul 30, 2024

As far as I understand self._api_version is defined but not used.

It would nice, if there was a check on functionalities that are not available in all version.
If the api version is outdated, return NotImplementedError or even better a custom "NotImplementedOnAPI" for those methods.

Example:
FirecREST v1.15.0 for example, doesn't support recursive ls.
in this case, if pyfirecrest is instantiated, with self._api_version=1.15.0 it should return NotImplementedOnAPI when list_files(recursive =True)

(this issue is indirectly related to eth-cscs/firecrest#204)

@ekouts
Copy link
Collaborator

ekouts commented Jul 31, 2024

Hm, indeed right now pyfirecrest doesn't really use this much. The only case where it is used, is for some incompatibility issue with the object storage link in external transfers. When an endpoint is new then in theory the client can easily catch the 404 result from the server, but when new arguments are added then they are silently ignored (as you mentioned in the recursive ls).
I could go through the different versions' release notes and implement this, but to be honest it's a bit low in my current priorities and with the holiday season I would say that this may be ready by the end of August. I hope this is okay 😅

@khsrali
Copy link
Contributor Author

khsrali commented Jul 31, 2024

I opened a PR on this, only because already had a look, and was half the way through...
If you don't like the approach, feel free to solve it in a different way :)
But I'm not in a hurry no worries :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants