refactor: move appstate logic to modules by taimoorzaeem · Pull Request #5090 · PostgREST/postgrest

taimoorzaeem · 2026-07-10T20:05:29Z

The problem occurs when solving #4894.

The way things were setup, we would have to move all the Listener.hs logic to AppState.hs just how all retryingSchemaCacheLoad logic present in AppState.hs even though it doesn't belong to it. Without this, we run into circular dependency errors.

This PR offers a module structure where all reloading logic is in Reload.hs including schema cache reload, config reload and listener retrying (and later reloading it).

Looking for feedback!

Notes

Related: Restructuring PostgREST source code #4952

wolfgangwalther · 2026-07-11T19:28:02Z

 import System.Directory (doesPathExist)

-run :: AppState -> Weak ThreadId -> IO ()
+run :: AppState.AppState -> Weak ThreadId -> IO ()


Did not review all of the PR (almost nothing in fact), but this caught my eye: This pattern of importing the type without prefix and then using all the functions from that module with prefix was introduced on purpose.

Do we really need to go to AppState.AppState? I don't think so.

The underlying idea is: use a prefix if it's not immediately clear what we're referring to without a prefix. There is no other AppState than AppState.AppState.

Yeah, we don't need AppState.AppState. Now that I have changed to re-exporting AppState type, we don't need to touch this at all. 👍

wolfgangwalther

Personally, I think I consider Pool and Reload to be part of AppState - but I see how you don't want to put everything into one big AppState file.

I don't really like the idea of having a AppState.Types file and importing that everywhere else - I feel like this (the Types file) is an implementation detail of the (possibly split across files) AppState module.

How about:

re-export the AppState type from AppState.hs and use that as before in all other files.
move Pool and Reload into AppState/ and have these two import the "internal" Types file to avoid the cycle.
Still have AppState re-export whatever is needed elsewhere from Pool / Reload.

Have only spent like 5 minutes thinking about this, so maybe this doesn't work or maybe there is a better way.

wolfgangwalther · 2026-07-15T19:59:27Z

+                      PostgREST.AppState.Pool
+                      PostgREST.AppState.Reload
+                      PostgREST.AppState.Types


I think it'd make sense to start adding the other-modules section for these. We'll control what to actually expose via PostgREST.AppState.

Hm, agree. I'll open another issue to discuss not exposing internal modules.

EDIT: Opened #5105 to discuss that.

wolfgangwalther · 2026-07-16T19:56:01Z

-- | Run an action with a database connection.
-usePool :: AppState -> SQL.Session a -> IO (Either SQL.UsageError a)
-usePool appState@AppState{stateObserver=observer, ..} sess = do


The removal of usePool is in the 3rd commit, but should be in the 2nd commit where it is added in the other file.

Maybe make sure that each commit still builds (and passes postgrest-coverage as well) on its own?

Ah, you're right. Will do.

I noticed we run weeder only on postgrest-coverage.

postgrest/nix/tools/tests.nix

Lines 174 to 177 in f81dd29

(

trap 'echo Found dead code: Check file list above.' ERR ;

${weeder}/bin/weeder --config=./test/weeder.toml

)

It's easy to miss running postgrest-coverage, how about we run weeder as part of postgrest-lint instead? After all. it is a dead code detector so it should be part of lint script. WDYT?

I think there was a reason for it. Does weeder need some of the build artifacts or so?

The weeder readme says:

Weeder uses HIE files produced by GHC - these files can be thought of as source code that has been enhanced by GHC, adding full symbol resolution and type information.

Weeder uses HIE files produced by GHC - these files can be thought of as source code that has been enhanced by GHC, adding full symbol resolution and type information.

Hm, so that means we can't include it in the postgrest-lint because that is supposed to only lint static source files without the need of any additional build artifacts? Makes sense.

I low-key still wish if we could have a separate postgrest- script for checking dead code, we could also include it in postgrest-check.

`AppState.hs` is imported by many modules. Some of those modules contain important functions that need to be imported back to `AppState.hs`, causing circular dependency problem. Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>

Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>

This commit moves, schema cache reload, config reload and listener reload logic to `Reload.hs` module. Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>

taimoorzaeem added the hygiene cleanup or refactoring label Jul 11, 2026

taimoorzaeem marked this pull request as draft July 11, 2026 08:33

taimoorzaeem force-pushed the refactor/move-appstate-logic branch from 952489e to 7309024 Compare July 11, 2026 08:34

wolfgangwalther reviewed Jul 11, 2026

View reviewed changes

taimoorzaeem force-pushed the refactor/move-appstate-logic branch from 7309024 to 7fd8f5e Compare July 15, 2026 19:40

wolfgangwalther reviewed Jul 15, 2026

View reviewed changes

taimoorzaeem force-pushed the refactor/move-appstate-logic branch from 7fd8f5e to 7504b65 Compare July 15, 2026 20:18

taimoorzaeem marked this pull request as ready for review July 15, 2026 20:23

taimoorzaeem marked this pull request as draft July 16, 2026 08:35

wolfgangwalther reviewed Jul 16, 2026

View reviewed changes

taimoorzaeem force-pushed the refactor/move-appstate-logic branch from 7504b65 to 97509a9 Compare July 17, 2026 11:09

taimoorzaeem marked this pull request as ready for review July 17, 2026 11:14

taimoorzaeem mentioned this pull request Jul 17, 2026

Don't expose internal haskell source modules #5105

Open

taimoorzaeem force-pushed the refactor/move-appstate-logic branch from 97509a9 to 538b53a Compare July 17, 2026 18:04

taimoorzaeem added 3 commits July 20, 2026 22:57

refactor: move connection pool functions to Pool.hs

4db9828

Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>

refactor: move reloading related logic to Reload.hs

09e7609

This commit moves, schema cache reload, config reload and listener reload logic to `Reload.hs` module. Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>

taimoorzaeem force-pushed the refactor/move-appstate-logic branch from 538b53a to 09e7609 Compare July 20, 2026 18:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor: move appstate logic to modules#5090

refactor: move appstate logic to modules#5090
taimoorzaeem wants to merge 3 commits into
PostgREST:mainfrom
taimoorzaeem:refactor/move-appstate-logic

taimoorzaeem commented Jul 10, 2026 •

edited

Loading

Uh oh!

wolfgangwalther Jul 11, 2026

Uh oh!

taimoorzaeem Jul 15, 2026

Uh oh!

wolfgangwalther left a comment

Uh oh!

wolfgangwalther Jul 15, 2026

Uh oh!

taimoorzaeem Jul 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

wolfgangwalther Jul 16, 2026

Uh oh!

taimoorzaeem Jul 16, 2026

Uh oh!

wolfgangwalther Jul 16, 2026

Uh oh!

wolfgangwalther Jul 16, 2026

Uh oh!

taimoorzaeem Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

	(
	trap 'echo Found dead code: Check file list above.' ERR ;
	${weeder}/bin/weeder --config=./test/weeder.toml
	)

Uh oh!

Uh oh!

Conversation

taimoorzaeem commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wolfgangwalther left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taimoorzaeem Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

taimoorzaeem commented Jul 10, 2026 •

edited

Loading

taimoorzaeem Jul 15, 2026 •

edited

Loading