-
Notifications
You must be signed in to change notification settings - Fork 43
Core-977 - inject GitHub username into lambda handler and call Mixpanel API directly #855
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
Conversation
✅ Deploy Preview for pensive-meitner-faaeee ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small, non-blocking nit, but otherwise LGTM
|
|
||
| def lambda_handler(event, context): | ||
| return "Hello from Gruntwork!" | ||
| url = "https://api.mixpanel.com/track" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👨🏻🍳 🤌 excellent
scripts/module-tutorial-client.js
Outdated
| githubUsername = 'unknown' | ||
| } | ||
| // Rewrite the %unknown% placeholder in the main.py configuration on the tutorial page | ||
| document.body.innerHTML = document.body.innerHTML.replace('%unknown%', githubUsername); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slick!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great. Did you check in package-lock.json by accident, or is that a bi-product of something else?
Ah yeah - that was actually intended: in the docusaurus config file, to load the new client module script, I needed to call Not sure yet if current build errors are still related to that or not. Yesterday and the day before, Netlify logs were showing me an auth error where it looked like Netlify was unable to fetch the repo itself during the build 🤔 |
- These changes examine the URL's query string params for a github_username and inject it, along with our tutorial service's endpoint, into the user's demo lambda handler code
-Use requests bundled with lambda to avoid dependencies - Fix call to extremely tempermental Mixpanel API
a77fd52 to
8281a9a
Compare
|
Thanks for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Relates to: https://github.com/gruntwork-io/dogfood-infrastructure-modules/pull/150
These changes examine the URL's query string params for a github_username and inject it, along with our tutorial service's endpoint, into the user's demo lambda handler code.
The injections are implemented by a new custom client-side script included in this PR. The client side script is designed to only run on our "Deploying your first module" tutorial page, so it should theoretically have minimal impact on the operation of the rest of the docs site.
The injected endpoint is our GW tutorial service's API gateway deployment URL. The customer's demo lambda's Python handler code has also been updated in this PR to call this specific endpoint and return its response, thus completing the round-trip and book-keeping for tracking activations / users who reached our tutorial goal state.
Ideally, I'd like to figure out how to properly define our GW tutorial service's endpoint solely in
docusaurus.config.jsand load it from within the custom client script that handles the value injection on our "Deploying your first module" , but currently when I attempt to follow docusaurus's docs on loading docusaurus config values I get an error about misusing React hooks. I'd like to manage this config centrally so that future PRs to update the endpoint could be one line change.