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: APNs from the ground up #100

Merged
merged 30 commits into from
May 18, 2024
Merged

Rewrite: APNs from the ground up #100

merged 30 commits into from
May 18, 2024

Conversation

JJTech0130
Copy link
Owner

@JJTech0130 JJTech0130 commented May 11, 2024

Trying to get the ball rolling here, this is my proposal for an initial structure of the rewrite.

I've structured it so that APNs will eventually be published as it's own independent PyPI package.
The first stage of pypush's rewrite will be to rip everything out, and polish off just APNs by itself.

Still need:

  • Rewrite actual APNs implementation, using asyncio this time
  • Work out API for device details and versions

@JJTech0130
Copy link
Owner Author

Maybe this time I should go ahead and implement #61 right off the bat

@JJTech0130
Copy link
Owner Author

Here's how the package structure and API is looking:

pypush-apns and pypush are the package names for now. Might need to be renamed if I can't get pypush on pypi
pypush depends on pypush-apns, but APNs can be used independently

The namespace means you can use it like this:

import pypush.apns

no matter how you install it

@JJTech0130
Copy link
Owner Author

Using asyncio over trio for the added API flexibility, though it does bring some additional pitfalls and possible footguns...

Can now provide 2 APIs:

@pytest.mark.asyncio
async def test_connect():
    connection = apns.connection.Connection(certificate, key)
    await connection.connect()
    assert connection.connected == True
    await connection.aclose()
    assert connection.connected == False

and

@pytest.mark.asyncio
async def test_with_block():
    async with apns.connection.Connection(certificate, key) as connection:
        assert connection.connected == True
    assert connection.connected == False

I store both the event loop and a list of tasks to cancel inside Connection, I'm not sure if that is the best way to use asyncio?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
apns/pyproject.toml Outdated Show resolved Hide resolved
@JJTech0130
Copy link
Owner Author

Honestly probably going to stick with recommending
pip install git+https://github.com/JJTech0130/pypush
rather than deal with publishing to PyPi for now

(pip install git+https://github.com/JJTech0130/pypush@initial-rewrite-structure for this branch)

@BuyAppleidinbulk

This comment was marked as spam.

@JJTech0130
Copy link
Owner Author

JJTech0130 commented May 15, 2024

apnsproxy is now working: it allows you to see the APNs packets that your local machine is sending over the wire. Note that it requires SIP to be disabled. (It only works on macOS, obviously)

python3.11 -m venv venv
source ./venv/bin/activate

pip install 'pypush[proxy] @ git+https://github.com/JJTech0130/pypush@initial-rewrite-structure'
sudo apnsproxy

@JJTech0130
Copy link
Owner Author

Just refactored the CLI, the command is now just pypush, pypush proxy replaces the apnsproxy command.

@JJTech0130 JJTech0130 marked this pull request as ready for review May 17, 2024 20:41
@JJTech0130 JJTech0130 changed the title Rewrite: Initial structure Rewrite: APNs from the ground up May 17, 2024
@JJTech0130 JJTech0130 mentioned this pull request May 17, 2024
@JJTech0130 JJTech0130 mentioned this pull request May 17, 2024
@JJTech0130 JJTech0130 linked an issue May 17, 2024 that may be closed by this pull request
self._tg.start_soon(self._receive_task)
ack = await self.receive(protocol.ConnectAck)
logging.debug(f"Connected with ack: {ack}")
assert ack.status == 0

Choose a reason for hiding this comment

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

Might be worth implementing custom exceptions to differentiate between problems that might occur, and potentially actuate different resilience strategies.

Copy link
Owner Author

@JJTech0130 JJTech0130 May 18, 2024

Choose a reason for hiding this comment

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

My strategy right now has pretty much been "if something goes wrong, catch it with except Exception as e and try again"... perhaps some proper exceptions can go in the next PR



@auto_packet
@dataclass

Choose a reason for hiding this comment

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

Since every class decorated by auto_packet is also a dataclass, can we potentially consolidate the two decorators by applying the dataclass decorator within the auto_packet implementation?

Copy link
Owner Author

Choose a reason for hiding this comment

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

this is actually harder than it sounds, I tried it and it messed up type checking...

pypush/apns/_protocol.py Outdated Show resolved Hide resolved
pypush/apns/_protocol.py Outdated Show resolved Hide resolved
@JJTech0130 JJTech0130 merged commit b1c30a9 into main May 18, 2024
@JJTech0130 JJTech0130 deleted the initial-rewrite-structure branch May 18, 2024 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python 3.12 support
4 participants