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

Dynamically load api config and refactor the logger #51

Merged
merged 12 commits into from
Oct 1, 2023

Conversation

andreogle
Copy link
Member

This branch originally started out for getting the Cloudformation file setup, but I ended up using it for refactoring the config files based on the discussions. This work can continue separately.

Changes

  1. Removes logger from both config files in favour of env variables
  2. The api package will now dynamically fetch and cache the logger based on CONFIG_SOURCE. The data-pusher will follow the same pattern later.
  3. Makes the logger usage consistent in across both packages

Let me know what you think

@andreogle andreogle requested a review from Siegrift September 27, 2023 13:36
@andreogle andreogle self-assigned this Sep 27, 2023
Copy link
Collaborator

@Siegrift Siegrift left a comment

Choose a reason for hiding this comment

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

I actually quite liked that the logger configuration was specified in the config file, because everything was configured in a single place.

I see the issue of loading file before initializing logger, but it is not a big deal considering we want to exit the app when config loading fails.

packages/common/src/logger/index.ts Show resolved Hide resolved
packages/data-pusher/src/api-requests/signed-api.test.ts Outdated Show resolved Hide resolved
packages/api/src/utils.ts Outdated Show resolved Hide resolved
@Siegrift Siegrift force-pushed the feature/cloudformation branch from af9dca4 to 915c4f7 Compare October 1, 2023 09:31
@Siegrift
Copy link
Collaborator

Siegrift commented Oct 1, 2023

@andreogle I've made two notable changes.

  1. Fix TS configuration by removing composite which resulted in outdated TS cache. The composite build is setup on Airnode, which is much larger and we don't need such setup here.
  2. Improve validation around the ENVs. We want to fail asap when the user supplies invalid configuration. This is done by flowing the ENVs through zod schema. This way, we also get the ENVs in a more typesafe way (and can apply defaults). Using the loadEnv should be used everytime you need some ENV from process.

I will document the configuration in #56

@Siegrift Siegrift force-pushed the feature/cloudformation branch from 915c4f7 to ce26d48 Compare October 1, 2023 09:38
@Siegrift Siegrift force-pushed the feature/cloudformation branch from ce26d48 to 9ed1d4b Compare October 1, 2023 09:41
@Siegrift Siegrift merged commit 6bfb012 into main Oct 1, 2023
3 checks passed
@Siegrift Siegrift deleted the feature/cloudformation branch October 1, 2023 09:44
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 this pull request may close these issues.

2 participants