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

Rewrite Query response class to #306's style #536

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

PerchunPak
Copy link
Member

I do not propose to release it in v11, as we probably won't merge it in time.

I made d8b26f1 as separate commit in case if you will want to release it with v11, while the main commit here will stay here for a while.


This was easier than I expected actually, most of the work was done in #306.

@PerchunPak PerchunPak added area: API Related to core API of the project type: rewrite Complete or partial rewrite of a part of the codebase labels May 8, 2023
@PerchunPak PerchunPak force-pushed the rewrite-query-answer-class branch 2 times, most recently from d68adea to 9a2e743 Compare May 14, 2023 19:28
@PerchunPak PerchunPak requested a review from ItsDrike as a code owner May 29, 2023 19:21
@PerchunPak PerchunPak added status: needs review Author is waiting for someone to review and approve status: stale Has had no activity for a while labels Jun 22, 2023
@PerchunPak PerchunPak requested review from ItsDrike and kevinkjt2000 and removed request for ItsDrike August 14, 2023 01:12
Will be removed 2023-12, use :attr:`.name` instead.
"""
return self.name
warnings.warn("`mcstatus.status_response` is deprecated, use `mcstatus.responses` instead", DeprecationWarning, stacklevel=2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use utils.deprecated like this instead of manually calling warnings.warn

Suggested change
warnings.warn("`mcstatus.status_response` is deprecated, use `mcstatus.responses` instead", DeprecationWarning, stacklevel=2)
deprecated("mcstatus.status_response", replacement="mcstatus.responses", date="2023-12", msg="module was renamed")()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mcstatus.utils.deprecated doesn't work with strings:

  File "/home/perchun/programming/mcstatus/mcstatus/utils.py", line 156, in decorate
    name = getattr(obj, "__qualname__", obj.__name__)
                                        ^^^^^^^^^^^^
AttributeError: 'str' object has no attribute '__name__'. Did you mean: '__ne__'?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer relevant, since #687 was merged

mcstatus/querier.py Outdated Show resolved Hide resolved
mcstatus/querier.py Outdated Show resolved Hide resolved
mcstatus/querier.py Show resolved Hide resolved
mcstatus/status_response.py Outdated Show resolved Hide resolved
katrinafyi added a commit to rina-forks/mcstatus that referenced this pull request Jul 20, 2024
PerchunPak added a commit that referenced this pull request Jul 27, 2024
* fix crash with one command-line argument

* implement ping() on BedrockServer

simply measures the latency of status()

* support Bedrock servers in CLI

done in a slightly ad-hoc way, but this is the best
we can do given the split of the response types.

* print server kind and tweak player sample printing

* JavaServer ping() doesn't work?

* fix precommit warnings

* review: remove Bedrock ping()

* review: change CLI ping comment to be more permanent

* review: formalise hostip/hostport within QueryResponse

* review: only squash traceback in common errors

* review: leading line break for multi-line motd

* Revert "review: formalise hostip/hostport within QueryResponse"

This reverts commit 3a0ee8c.

* review: use motd.to_minecraft() in json

* review amendment: factor out motd line breaking

* review: refactor CLI json() to use dataclasses.asdict()

* amendment: add NoNameservers and remove ValueError from squashed errors

ValueError might be thrown by programming errors in
json handling, for example.

* review: fallback logic in CLI ping

since this runs both ping() then status(), it can report
precisely when one fails and the other succeeds.

some kludgy logic to switch bedrock too.

* review: use ip/port fields in CLI's JSON output

in anticipation of #536

Co-authored-by: Perchun Pak <[email protected]>

* review: avoid kind() classmethod

* review: clarify MOTD serialisation comment

* review: simplify ping fallback logic

Co-authored-by: Perchun Pak <[email protected]>

* make version consistent between status and query

* review: apply simplify() to motd in CLI JSON output

Co-authored-by: Perchun Pak <[email protected]>

* review: use separate JSON field for simplified MOTD

* review: remove MOTD fixup comment

* review: update README with new CLI

* review: no raw motd

* no --help output in readme

* review: allow main() with no arguments

* Update mcstatus/__main__.py

Co-authored-by: Kevin Tindall <[email protected]>

* avoid json collision

* oops! good linter

* drike review

* good linter

* one more ci failure and i turn on the computer

* also squash ConnectionError

happens during server startup, for example

---------

Co-authored-by: Perchun Pak <[email protected]>
Co-authored-by: Kevin Tindall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Related to core API of the project status: needs review Author is waiting for someone to review and approve status: stale Has had no activity for a while type: rewrite Complete or partial rewrite of a part of the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants