Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the way we enable dev mode in configuration to validate changes #1811

Closed
bnjjj opened this issue Sep 16, 2022 · 0 comments · Fixed by #1813
Closed

Change the way we enable dev mode in configuration to validate changes #1811

bnjjj opened this issue Sep 16, 2022 · 0 comments · Fixed by #1813
Assignees

Comments

@bnjjj
Copy link
Contributor

bnjjj commented Sep 16, 2022

Possibly in the constructor here before the call to validate: https://github.com/apollographql/router/blob/main/apollo-router/src/configuration/mod.rs#L260

Then we can get rid of the multiple times this is called in ConfigurationSource.
Also it means that dev mode is set up before validation takes place. Setting it after means we potentially miss validation errors that could be caused by the activation of dev mode.

Originally posted by @BrynCooke in #1808 (comment)

@abernix abernix added this to the v1.0.0-rc.1 milestone Sep 16, 2022
bnjjj added a commit that referenced this issue Sep 16, 2022
close #1811 

I changed the way we watched file changes. When we launch the file
watching it automatically triggers a first event, so before we were
reading the configuration twice. Now only once.

I had to introduce `serial_test` crate to force 2 tests for dev mode to
be ran in serial, because it needs to check the environment variable.
It's a temporary fix to let me test it and be ready for the release. But
I will find another solution in the future

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants