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

fix: move references to amplitude key to after .env loading #177

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Conversation

Sparkier
Copy link
Member

Previously, the environment variable containing the amplitude API key was referenced before loading the .env file. This has been changed so that the key does not have to be set outside the .env file.

Previously, the environment variable containing the amplitude API key was referenced before loading the .env file. This has been changed so that the key does not have to be set outside the .env file.
@Sparkier
Copy link
Member Author

I also think that we should make this key entirely optional since people might want to deploy without using amplitude.

Created a linear issue.

@cabreraalex
Copy link
Member

idk if we want to initialize amplitude in each server call? probably only want to start it when the server starts

@Sparkier
Copy link
Member Author

idk if we want to initialize amplitude in each server call? probably only want to start it when the server starts

I agree, but if we do it like now the server crashes if the env variable is not set since we try to access it before loading the .env file. So this is more of a hot fix for that.

@Sparkier
Copy link
Member Author

What we could do instead is check for that env variable before initializing amplitude and only log things when it's set. This would also not log events when developing locally and might be the preferable option.

@Sparkier
Copy link
Member Author

I changed the implementation to use a singleton object. This also checks if there is a tracking key at all.

@Sparkier Sparkier merged commit 8948955 into main Sep 18, 2023
6 checks passed
@Sparkier Sparkier deleted the env_key branch September 18, 2023 15:07
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