cathaypacific8747 / fr24

Download and parse data from flightradar24.com with gRPC/JSON
https://cathaypacific8747.github.io/fr24/
MIT License
16 stars 6 forks source link

Authentication issues #19

Closed xoolive closed 6 months ago

xoolive commented 7 months ago

I don't have time to dig much more into this right now, so let me document it here first before I forget. I currently have some authentication issues (context: trying to port my code from the older version to the newer API, and struggling to find where I could just get the JSON before the pyarrow...)

More generally, it would be nice to have an extra option as I could prefer to run anonymously by default, and login only when necessary:

async with FR24(login: bool | {'username': user, 'password': pass}) as fr24:
    ...

Currently, on my side, authentication in the asynccontextmanager fails with

ConnectError: All connection attempts failed

Surprisingly, in the TUI part of the code, I just use the following (without the FR24 client) and it still works:

self.auth = await login(self.client)
cathaypacific8747 commented 7 months ago

I am unable to reproduce the ConnectError on my end, but I suspect the issue is due to the custom httpx.AsyncHTTPTransport(retries=5) passed into the httpx.AsyncClient here. The documentation for the retry logic is rather lacking, so there might be something I'm overlooking. Does removing it resolve the issue?

As for how to get the JSON before pyarrow, you could do this:

from fr24.core import FR24

async def my_playback() -> None:
    async with FR24() as fr24:
        pb = fr24.playback(flight_id=0x2FB3041)
        response = await pb.api.fetch() # raw json here
        pb.data.add_api_response(response) # transformed to arrow table

await my_playback()

I'll have a go at the proposed anonymous-by-default implementation :)

cathaypacific8747 commented 7 months ago

Just implemented the authentication override:

# reads from environment variables/config file by default
async with FR24() as fr24:
    ...
# force anonymous
async with FR24(creds=None) as fr24:
    ...
async with FR24(creds={"username": "...", "password": "..."}) as fr24:
    ...
async with FR24(creds={"subscriptionKey": "...", "token": "..."}) as fr24:
    ...
xoolive commented 7 months ago

Indeed, if I comment the transport from the HTTPClient it works. I do wonder: is it really necessary to have this argument by default? Why not having a **kwargs argument in HTTPClient (and FR24) if people want to add anything to the client (for example a transport parameter?)

async with FR24(transport=AsyncHTTPTransport(retries=5)) as fr24: ...

I had an extended thought about the async context manager for FR24, and I am really circumspect about the automatic login by default.

What do you think of explicit login, something like:

async with FR24() as fr24:
    await fr24.login()  # with the usual login
    await fr24.whatever()

Personally, I would feel more comfortable like that. That would make the creds argument you just created irrelevant though :sweat_smile:

Depending on what you do with your script (also with tests), you might prefer to be logged out by default (and only login on demand when necessary?)

xoolive commented 7 months ago

Actually, more general architecture concern: how to proceed if I want to come with my own httpx.AsyncClient because I want to share it with other tools?

cathaypacific8747 commented 7 months ago

I had been using AsyncHTTPTransport(retries=5) because I was intermittently receiving connection errors particularly during livefeed requests: it wasn't really necessary. I had a quick glance at the default AsyncHTTPTransport created by the httpx.AsyncClient but was unable to find any differences with our custom one.

Anyways, I've made a commit that allows you to pass a custom client and made the login explicit:

client = httpx.AsyncClient(transport=httpx.AsyncHTTPTransport(retries=5))
async with FR24(client) as fr24:
    # anonymous by default
    await fr24.http.login() # reads from environment or configuration file, or,
    await fr24.http.login(creds={"username": "...", "password": "..."}) # or,
    await fr24.http.login(creds={"subscriptionKey": "...", "token": "..."})

This is a breaking change for my codebase as it no longer autologins, but yeah, explicit logins do look nicer.