Skip to content
This repository has been archived by the owner on Feb 28, 2023. It is now read-only.

rouille -> warp #131

Merged
merged 25 commits into from
Jan 10, 2022
Merged

rouille -> warp #131

merged 25 commits into from
Jan 10, 2022

Conversation

montekki
Copy link
Contributor

Still a lot of work to do but I think that api will be compatible with old code.

Fixes #39

@montekki montekki changed the title Initial commit rouille -> warp Dec 20, 2021
@montekki montekki marked this pull request as ready for review December 28, 2021 10:17
@drahnr
Copy link
Contributor

drahnr commented Jan 3, 2022

I'll give this a spin at some point this week, other than that this is looks good to me and ready to be merged 🎉

@drahnr
Copy link
Contributor

drahnr commented Jan 7, 2022

cachepot -s

yields

cachepot: error: failed to get stats from server
cachepot: caused by: Failed to send data to or receive data from server
cachepot: caused by: Failed to read response header
cachepot: caused by: failed to fill whole buffer

with the local dispatch ( or local server) logs:

[2022-01-07T09:32:46Z DEBUG cachepot::config] Attempting to read config file at "/home/bernhard/.config/cachepot/config"
[2022-01-07T09:32:46Z DEBUG cachepot::config] Couldn't open config file: failed to open file `/home/bernhard/.config/cachepot/config`
[2022-01-07T09:32:46Z INFO  cachepot::config] Using the default configuration.
[2022-01-07T09:32:46Z INFO  cachepot::server] start_server: port: 4226
[2022-01-07T09:32:46Z INFO  cachepot::server] No scheduler address configured, disabling distributed cachepot
[2022-01-07T09:32:46Z DEBUG cachepot::cache::cache] Trying Redis(redis://127.0.0.1:6379/7)
[2022-01-07T09:32:46Z INFO  cachepot::server] server started, listening on port 4226
[2022-01-07T09:32:49Z DEBUG cachepot::server] handle_client: get_stats
[2022-01-07T09:32:49Z ERROR cachepot::server] Failed to bind socket: Connection refused (os error 111)

@montekki
Copy link
Contributor Author

montekki commented Jan 7, 2022

Were all previously launched instances of daemonized cachepot killed beforehand?

@drahnr
Copy link
Contributor

drahnr commented Jan 7, 2022

Were all previously launched instances of daemonized cachepot killed beforehand?

Double checked:

After running sudo systemctl restart cachepot-server.service cachepot-scheduler.service; sudo pkill cachepot; ps ax | rg cachepot with only the rg process itself, same result.

Imho this error message is a bug in itself, it provides insufficient context: [2022-01-07T09:32:49Z ERROR cachepot::server] Failed to bind socket: Connection refused (os error 111)

@montekki
Copy link
Contributor Author

montekki commented Jan 7, 2022

Damn, I cant repro and i didn't even touch src/server.rs (i think)

@montekki
Copy link
Contributor Author

montekki commented Jan 7, 2022

Can you CACHEPOT_LOG=trace?

@montekki
Copy link
Contributor Author

montekki commented Jan 7, 2022

Also, I've run into ufw blocking connects

@drahnr
Copy link
Contributor

drahnr commented Jan 7, 2022

I think my local redis just crashed and this is a side effect of it. I'll try to fix the instance, and open an issue regarding error handling of the backend if this is resolved.

@Xanewok
Copy link
Contributor

Xanewok commented Jan 7, 2022

cachepot -s

yields

<error>

I can't repro, I'm getting regular stat dump using this branch

@drahnr
Copy link
Contributor

drahnr commented Jan 7, 2022

After fixing redis, the output is as expected.

src/util.rs Show resolved Hide resolved
Copy link
Contributor

@Xanewok Xanewok left a comment

Choose a reason for hiding this comment

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

Excited for this change! 🎉 Looks good at a glance, I'll dig deeper later on since I had a hard time following the logic bundled together in a single file

src/dist/http.rs Show resolved Hide resolved
}
}

mod scheduler_api_v1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it'd makes to extract API routes/logic to separate files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would suggest in another changeset.


impl warp::reject::Reject for Error {}

pub(super) mod filters {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto wrt modules

}
}

pub(super) mod handlers {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@montekki montekki merged commit 943fc53 into master Jan 10, 2022
@montekki montekki deleted the fs-remove-rouille branch January 10, 2022 09:35
@montekki
Copy link
Contributor Author

Merged this since it is blocking all stuff that needs to be done on top of it. In case there (most likely) be fallout lets deal with it iteratively in new issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move away from rouille
3 participants