Skip to content

Enhancing the deployment experience #443

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.

Sign up for GitHub

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

Jump to bottom
Merged
jainr merged 14 commits into from
Jul 12, 2022
Merged

Enhancing the deployment experience #443

jainr merged 14 commits into from
Jul 12, 2022

Conversation

@jainr
Copy link
Collaborator

@jainr jainr commented Jul 7, 2022

  1. Currently the user needs to pass the Principal Id as a pre-req, we now create a managed identity within the template.
  2. We also removed the bacpac URL paramter from UI, since that is something user should not need to worry about

Successful Deployment
image

jainr and others added 11 commits June 3, 2022 13:24
@jainr
Adding EventHub creation to the deployment script
b8a5ce7
@jainr
Merge branch 'linkedin:main' into main
c1ade5c
@jainr
Upgrading API versions and fixing the if clause of KV property creati…
8d162f0 
…on for EH
@jainr
Merge branch 'main' of github.com:jainr/feathr into main
c9bfe05
@jainr
Merge branch 'main' of https://github.com/linkedin/feathr into main
1a6161f
@jainr
Merge branch 'main' of https://github.com/linkedin/feathr into main
8d2de24
@jainr
Merge branch 'main' of https://github.com/linkedin/feathr into main
e961b4d
@jainr
Merge branch 'main' of https://github.com/linkedin/feathr into main
52b6181
@jainr
Merge branch 'main' of https://github.com/linkedin/feathr into main
9079d91
@jainr
Removing the need to ask for Principal Id from user and creating it i…
cb517f4 
…n the template. Also removing bacpac file url parameter, and copying it to user's storage account as part of the template. This simplifies the flow for user
@jainr
Making template more organized and updated documentaton
ddb8245
@jainr jainr requested review from Yuqing-cat, windoze and xiaoyongzhu and removed request for windoze and xiaoyongzhu July 7, 2022 21:29
xiaoyongzhu
xiaoyongzhu previously approved these changes Jul 8, 2022
View reviewed changes
@jainr jainr requested review from blrchen and edwincheung July 9, 2022 04:38
@blrchen
Copy link
Collaborator

blrchen commented Jul 11, 2022

Hello @jainr , I tried with this PR and the web app failed to start, could you please make following changes to make it align latest code

  1. Change name of app settings to following two, now UI and backend use different client id. These two are generated on the flying after ui app registration is completed, user does not need to provide
  • REACT_APP_AZURE_CLIENT_ID
  • REACT_APP_AZURE_TENANT_ID
  1. Change docker image name to feathrfeaturestore/feathr-registry. Now latest docker image supports selected registry scenario.

@jainr
Copy link
Collaborator Author

jainr commented Jul 11, 2022

Thanks Blair, I addressed your comments.

@jainr jainr merged commit 371444c into feathr-ai:main Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edwincheung edwincheung edwincheung approved these changes

@Yuqing-cat Yuqing-cat Yuqing-cat approved these changes

@xiaoyongzhu xiaoyongzhu xiaoyongzhu left review comments

@windoze windoze Awaiting requested review from windoze

@blrchen blrchen Awaiting requested review from blrchen

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jainr @blrchen @xiaoyongzhu @edwincheung @Yuqing-cat