feat(hogql): add readonly=1 and max_execution_time=60 on every query

[mariusandra]

Problem

HogQL queries might run out of bounds.

Changes

Implements two settings that will limit them somewhat:

• readonly=1 -- prevents any accidental changes going in
• max_execution_time=60 -- this is the default timeout for any HTTP requests

Instead of the previously discussed 30sec delay for HogQL, I set it to 60sec. That's because in practice I could get some more complex queries (e.g. slightly longer timeranges) on our team to exceeding the 30sec mark. Our HTTP timeouts are now at 60sec, so let's just roll with that. We can bring this down later.

I had discussed adding these settings with a feature flag (JSON payload), but decided against introducing more complexity for now. With feature flag JSON settings things get suspicious. For example, how do we sanitise them so that we don't add something to the end of every query that completely breaks the app for everyone? 🤔

How did you test this code?

• Wrote tests that verified the settings get added.
• Checked in the django dev output that queries still get executed and the settings get added

[pauldambra]

Question about the readonly setting...

I'll assume you had a good reason for the optional and ✅ to save a dance once you share it 🤣

[pauldambra]

unexpected snapshot change 🦐

[mariusandra]

This seems like something that should have changed on master too... 🤔

[pauldambra]

Do we want to be able to remove readonly? Should this be constant 1 forcing us to change code if we wanted to let HogQL do non-selects?

[mariusandra]

I guess we can change it then :)

[pauldambra]

So this is allowed

import dataclasses
from typing import Optional


@dataclasses.dataclass
class Thing:
    optional_value: Optional[int] = 1


x = Thing()
y = Thing(optional_value=None)

print(x.optional_value)
print(y.optional_value)

this prints

1
None

So, if we can accidentally or someone can maliciously get us to create settings with HogQLSettings(readonly=None) then we don't have readonly set on the query.

If we're saying that HogQL is readonly a constant here feels safer

[mariusandra]

Yeah, but none of the settings come from users --> we set them ourselves, and it's hardcoded in. I opted out of using feature flags to set these settings just for this reason. Now, if we want, we could, e.g. override some of them for some longer queries, e.g. setting max execution time to 5min for exports.

Theoretically we can also override readonly, but 1) nothing does that now, 2) users will not be able to set these settings... at least not directly, 3) it's still a select query so it's anyway an extra safety measure.

[mariusandra]

The "optional" keyword here means you don't need to specify a value when initialising the class. It'll still have the default value.

[pauldambra]

The "optional" keyword here means you don't need to specify a value when initialising the class. It'll still have the default value.

🤦