Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

feat(flags): Basic flags service #31

Merged
merged 11 commits into from
May 7, 2024
Merged

feat(flags): Basic flags service #31

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
aad0f2c
feat(flags): Basic flags service
neilkakkar May 1, 2024
b5591f5
format
neilkakkar May 1, 2024
b3308d3
rm old test
neilkakkar May 1, 2024
27e69e9
Merge branch 'main' of github.com:PostHog/hog-rs into flag-boilerplate
neilkakkar May 2, 2024
68fe2bb
clean add test
neilkakkar May 2, 2024
877e4df
rm not needed dep
neilkakkar May 2, 2024
b5c4ebf
Merge branch 'main' into flag-boilerplate
xvello May 3, 2024
777788f
fix lint
neilkakkar May 3, 2024
15a43e2
Merge branch 'main' into flag-boilerplate
neilkakkar May 7, 2024
1c3481f
address comment
neilkakkar May 7, 2024
3f3241e
Merge branch 'flag-boilerplate' of github.com:PostHog/hog-rs into fla…
neilkakkar May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions feature-flags/src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ pub fn router<R: Client + Send + Sync + 'static>(redis: Arc<R>) -> Router {
// .allow_credentials(true)
// .allow_origin(AllowOrigin::mirror_request());

let router = Router::new()
Router::new()
.route("/flags", post(v0_endpoint::flags).get(v0_endpoint::flags))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

endpoint name tbd

Copy link
Contributor

Choose a reason for hiding this comment

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

you'll need to wire readiness, liveness and metrics too. Can be done later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. Added to list here: PostHog/posthog#22131

.with_state(state);

router
.with_state(state)
}
2 changes: 1 addition & 1 deletion feature-flags/tests/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub struct ServerHandle {

impl ServerHandle {
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 tried not re-building this and just using axum-test, but ran into annoyances there around the IP extraction not working well, what capture does sans axum-test-helper indeed seems the most flexible + useful here 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

axum-test-helper is currently outdated and we switched to a fork but should move away from it anyway.
It's short enough to not bother trying to make it generic, let's just copy it as you did.

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 meant this one: https://docs.rs/axum-test/latest/axum_test/ but indeed control over all the boilerplate here, given its not a lot is 👍

pub async fn for_config(config: Config) -> ServerHandle {
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let listener = TcpListener::bind("127.0.0.1:3001").await.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Port zero is intentional, it tells the OS to assign a random port that's available, avoiding collision with other processes and other tests in parallel. That's why we then call let addr = listener.local_addr().unwrap(); to get the port to query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, great catch, thanks! I was trying something here, forgot to revert 🤦

let addr = listener.local_addr().unwrap();
let notify = Arc::new(Notify::new());
let shutdown = notify.clone();
Expand Down
Loading