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

meowlflow/serve: reuse app to inherit config #31

Merged
merged 1 commit into from
Dec 16, 2021
Merged

Conversation

squat
Copy link
Contributor

@squat squat commented Dec 16, 2021

Currently, the serve command instantiates its own FastAPI app, without
any of the configuration used in the sidecar.py module. This is a
problem because we miss out on the middleware and metrcs. This commit
changes the serve.py module so that it reuses the existing app.

Signed-off-by: Lucas Servén Marín [email protected]

Currently, the serve command instantiates its own FastAPI app, without
any of the configuration used in the sidecar.py module. This is a
problem because we miss out on the middleware and metrcs. This commit
changes the serve.py module so that it reuses the existing app.

Signed-off-by: Lucas Servén Marín <[email protected]>
@squat squat requested review from dswah and DKarakoc December 16, 2021 18:17
Copy link
Collaborator

@dswah dswah left a comment

Choose a reason for hiding this comment

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

nice catch! this looks simple enough!

@dswah
Copy link
Collaborator

dswah commented Dec 16, 2021

ah dang some build error. im on mobile so i cant see the error

@squat
Copy link
Contributor Author

squat commented Dec 16, 2021

just had to retry and it worked

we seem to be hitting something like this bug in Kaniko: GoogleContainerTools/kaniko#1592

Maybe we can switch from Kaniko to https://github.com/genuinetools/img

@squat squat merged commit 44f3920 into main Dec 16, 2021
@squat squat deleted the reuse_app_from_sidecar branch December 16, 2021 21:05
@dswah
Copy link
Collaborator

dswah commented Dec 16, 2021

nice work u little mergy guy!!

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