Open gwbischof opened 12 months ago
@gwbischof,
I like the idea of ditching the plemmy.responses
objects. I've been on the fence about them since they were implemented, and while I understand their necessity in JS/TS, they don't feel very "Pythonic".
I think it's worth considering whether to return plemmy.objects
classes directly from the LemmyHttp
object methods, perhaps from an extra optional argument:
from typing import Union
class LemmyHttp:
...
def get_community(self, id: int = None, name: str = None, as_http: bool = True) -> Union[requests.Response, plemmy.objects.Community]:
...
client = LemmyHttp("<srv>")
client.get_community(...) # current implementation, returns requests.Response
client.get_community(as_http=False) # new implementation, returns plemmy.objects.Community object
This way current projects utilizing Plemmy won't be affected by an update, Plemmy still gives the option to return API responses, and it simplifies using primitive objects (Community, Post, etc.).
Regarding something like community.create_post
or post.create_comment
: we could bake these functions inside their respective plemmy.objects
classes. For example:
# plemmy/objects.py
@dataclass
class Community:
... # dataclass vars
def create_post(self, name: str, ...):
plemmy.utils.post_handler(...)
One issue I see with this implementation is passing auth keys around (LemmyHttp.key
). Do Community
and Post
objects inherit the key? Is it just passed as an argument to specific methods (e.g., Community.create_post
)? There's a lot to consider if we want to ensure no "bloat" occurs.
For a PyMongo-like design/indexing pattern: this is very doable. For example, we could have a "private" LemmyHttp
class variable that houses, for example, a dictionary of communities, and class methods to simplify lookup:
class LemmyHttp:
def __init__(self, ...) -> None:
self.__communities = {}
def __getitem__(self, key) -> plemmy.objects.Community:
return self.__communities[key]
This could also be done for the Community
object in plemmy.objects
to index posts, or for the Post
object to list comments, etc. We do have to think about how the initial dictionaries of communities/posts/comments are populated - does the LemmyHttp
object query for all communities when it's initialized? Whenever a specific community is searched? Do Community
objects immediately query for all posts? Likewise for comments inside posts. (Edit: I think comments are already gathered when a post is queried, this shouldn't be an issue.) We also have to think about how much overhead this will cause on larger instances and/or instances federated with many, many communities. There may be other ways to implement these functions without too much overhead, let's think on this. (Edit: see comment below)
For immediate tasks, let's try prototyping returning plemmy.objects
classes (Community
, Post
, etc.) directly from LemmyHttp
methods without needing plemmy.responses
objects. I think this feature is a no-brainer (and I'm kicking myself for not thinking of it sooner!).
Best, Travis
Thinking/typing out loud here... what if, for PyMongo-like indexing, we only perform a query when we need to:
class LemmyHttp:
def __getitem__(self, key: str) -> plemmy.objects.Community:
return self.get_community(name=key, as_http=False) # this assumes previously-outlined changes
# if not found, we can throw an IndexError
client = LemmyHttp("<srv>")
community = client["<community>"]
Similarly for posts in communities:
# plemmy/objects.py
@dataclass
class Community:
... # dataclass vars
def __getitem__(self, post_id: int) -> plemmy.objects.Post:
... # perform query, format into Post object
post = community[<post_id>]
This is much more efficient than trying to house a dictionary/list of every community/post available to a given instance/community.
raise_for_status could be helpful, although I'm not sure if its the best choice.
Without raise_for_status:
In [38]: lemmy.get_community(name='community_that_doesnt_exist')
Out[38]: <Response [404]>
With raise_for_status:
In [39]: response = lemmy.get_community(name='community_that_doesnt_exist')
In [40]: response.raise_for_status()
HTTPError: 404 Client Error: Not Found for url: http://localhost:8536/api/v3/community?name=community_that_doesnt_exist&auth=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOjEsImlzcyI6ImxlbW15LWFscGhhIiwiaWF0IjoxNjkyOTI4NjY0fQ.rtbkMfZAMo--7jeawrzKvNhuDgaX-jn4B8M-Jfjo1Vs
It will give you an exception so that you can use normal python exception handling to deal with it.
I've laid the groundwork for returning plemmy.objects
objects: https://github.com/tjkessler/plemmy/commit/8e03e2ac90c37c564fc6e833d760a4bcb5d612f6
Also updated is the create_form
utility function to exclude as_http
arguments from form creation: https://github.com/tjkessler/plemmy/commit/556b4226bd9301e130c5c63f9f641c8a0cc101f4
I imagine we'll want to keep the plemmy.views
module around, it should make parsing objects from API responses much easier. Edit: maybe not, I'll try with and without.
Edit: let me know if you think any changes should be made to return types, there were some weird ones (like plemmy.get_modlog
) where I thought returning a dictionary was appropriate.
Because a post always has a community: https://github.com/LemmyNet/lemmy/blob/main/crates/db_schema/src/source/post.rs#L25 And a comment always has a post: https://github.com/LemmyNet/lemmy/blob/main/crates/db_schema/src/source/comment.rs#L24
Maybe the pattern that is used in pymongo would work well here, I think it would simplify using plemmy. https://pymongo.readthedocs.io/en/stable/tutorial.html And users wouldn't have to think about the plemmy.response types like GetComunityResponse
You could still do:
What do you think?