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.

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

Standalone sidecar charm #104

Merged
merged 11 commits into from
Mar 2, 2023
Merged

Standalone sidecar charm #104

merged 11 commits into from
Mar 2, 2023

Conversation

misohu
Copy link
Member

@misohu misohu commented Feb 23, 2023 •
edited
Loading

Deploy MLflow as a sidecar charm (without any relation).

MLflow is running and is exposed as NodePort on port speciffied in config.

This is a follow up PR on #103 (first review the 103).

Publish charm will fail as it is merging agains feature branch not main (but it works on main check this draft pr against main with the same code)

@misohu
KF-715
ba9d1d0 
Change CI to execute in AWS
Move the charm code to root folder
Remove examples folder
@misohu
KF-737
1fb2df3 
deploy mlflow as sidecar charm and expose it as nodeport speciffied in config
@misohu misohu requested a review from a team as a code owner February 23, 2023 13:01
@misohu
KF-737
9d03e62 
deploy mlflow as sidecar charm and expose it as nodeport speciffied in config
@misohu misohu marked this pull request as draft February 23, 2023 14:09
@misohu
KF-737
e9ee80e 
deploy mlflow as sidecar charm and expose it as nodeport speciffied in config
@misohu misohu mentioned this pull request Feb 24, 2023
Merged
@misohu misohu marked this pull request as ready for review February 27, 2023 13:54
DnPlas
DnPlas reviewed Mar 1, 2023
View reviewed changes
Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

Thansk @misohu, I reviewed commits 9d03e62 and e9ee80e, just left a comment.

metadata.yaml Show resolved Hide resolved
@misohu
Merge remote-tracking branch 'origin/KF-715-mlflow-v211' into KF-737-…
5a4f0b7 
…standalone-sidecar-charm
@misohu
KF-737
7c36209 
add idle_period after deploying charm in integration tests
@misohu
KF-737
52b1089 
add idle_period after deploying charm in integration test
@misohu
KF-737
fe6925e 
fix lint
@misohu
KF-737
9c80be0 
check integration test in AWS instance
@misohu
KF-737
398f256 
fix integration test
ca-scribner
ca-scribner reviewed Mar 1, 2023
View reviewed changes
src/charm.py Outdated Show resolved Hide resolved
ca-scribner
ca-scribner reviewed Mar 1, 2023
View reviewed changes
src/charm.py Outdated Show resolved Hide resolved
ca-scribner
ca-scribner reviewed Mar 1, 2023
View reviewed changes
src/charm.py Outdated Show resolved Hide resolved
ca-scribner
ca-scribner reviewed Mar 1, 2023
View reviewed changes
Copy link
Contributor

@ca-scribner ca-scribner left a comment

Choose a reason for hiding this comment

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

minor stuff, otherwise lgtm

@misohu
KF-737
7926bdb 
minor comments fixes plus typos
rename test case
refactor _create_service
DnPlas
DnPlas approved these changes Mar 2, 2023
View reviewed changes
Copy link
Contributor

@DnPlas DnPlas left a comment

Choose a reason for hiding this comment

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

Thanks @misohu, I think there are a few comments that we can resolve in a separate PR as this one is already very complex and convoluted. Please make sure to resolve the conversations of the status before we merge your dev branch into main.

@misohu misohu merged commit b642986 into KF-715-mlflow-v211 Mar 2, 2023
@misohu misohu deleted the KF-737-standalone-sidecar-charm branch March 2, 2023 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ca-scribner ca-scribner ca-scribner left review comments

@DnPlas DnPlas DnPlas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@misohu @DnPlas @ca-scribner