-
Notifications
You must be signed in to change notification settings - Fork 8
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
Feat azd compatible #9
Conversation
Co-authored-by: Pamela Fox <[email protected]>
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.
thank you!! cant remember why I didn't azd-ify, didnt expect a kind soul to do it for me. just a few changes requested.
DONE, I deleted my previous comment because I thought it was wrong but it's correct anyway you'll get it when you review the code now. TL;DR They use two ways to get the key EITHER implementing it just like I previously did (following their implementation I found that they were doing the same thing I was doing) OR using the SDK in python code to authenticate the application to access the key from the KV directly which I implemented here. Checkout the live application: https://test-env-final-t5awkbvbz57xg-appservice.azurewebsites.net/ I know that you might want to leave the core/security as is and do these things somewhere else but in that case, I will have to create another folder in the infra just for storing the keys in the key vault (just like Jon's folders). |
Thanks for all the changes! I will take a look soon. |
Hey @pamelafox, |
…ranslation-telephone into feat-azd-compatible
Let me know when you're ready for re-review, I have your branch checked out locally to do a test deploy. |
@pamelafox everything is good to go! I recently came across this amazing line that makes app service pull the secret from the key vault without storing it as plain text in the application configuration variables.
So, I was changing the code to work with it - which made some of the changes I made during the process of using a Key Vault unnecessary so, I removed stuff and tidied up everything for the latest update. I learned a lot. Thanks and Happy New Year to you and your family!! ❤️ |
Great! I deployed it and tweaked the readme a bit to emphasize migrating the DB. Merging, thanks so much! |
Thanks! |
Great to know! I spend most of my time these days in https://github.com/Azure-Samples/azure-search-openai-demo/ and we'll be running a hackathon around it in Jan/Feb, hope you can participate in that! It should be a lot of fun. (And there may be a prize for best OSS contribution...) |
I definitely will! |
Hey @pamelafox,
I love ❤️ your work and thought it would be a good thing to help you make this amazing project azd compatible. ✨
You'll still have to run the migrations yourself after the application is deployed. 🚀
I'm going to leave the application live for 48 hours if you want to test it out here you are two links
Deployment only with azd:
https://john-translation-phone-gmbxoafd3rnxo-appservice.azurewebsites.net/
Deployment with azd and migrations from SSH:
https://testing-env-nemljovihggx6-appservice.azurewebsites.net/recent
In this pull request: