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

feat/app-deployment-plugin #390

Merged
merged 31 commits into from
Jun 20, 2024
Merged

feat/app-deployment-plugin #390

merged 31 commits into from
Jun 20, 2024

Conversation

athul-rs
Copy link
Contributor

@athul-rs athul-rs commented Jun 6, 2024

What

  • App Deployment moved out of the code base.

Why

  • Needs to be a plugin feature.

How

  • Converting earlier used code to be present as a plugin and using a feature flag
  • Add generic APIs to list and evaluate feature flags

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • Yes, codes are removed from frontend.

Database Migrations

  • N/A

Env Config

  • N/A

Relevant Docs

  • N/A

Related Issues or PRs

  • N/A

Dependencies Versions

  • N/A

Notes on Testing

  • Done an app run-up and tested basic functionality after removal of the app deployment code.

Screenshots

image

Checklist

I have read and understood the Contribution Guidelines.

@athul-rs athul-rs requested review from johnyrahul, hari-kuriakose, a team and tahierhussain and removed request for a team June 6, 2024 20:13
Copy link
Contributor

@vishnuszipstack vishnuszipstack left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@johnyrahul johnyrahul left a comment

Choose a reason for hiding this comment

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

LGTM

@athul-rs athul-rs self-assigned this Jun 7, 2024
@athul-rs athul-rs changed the title feat/app-deployment-plugin feat/app-deployment-plugin ( NOT-TO-BE-MERGED ) Jun 10, 2024
@athul-rs athul-rs changed the title feat/app-deployment-plugin ( NOT-TO-BE-MERGED ) feat/app-deployment-plugin ( TO-BE-MERGED Later) Jun 10, 2024
@athul-rs athul-rs requested a review from johnyrahul June 11, 2024 20:12
@athul-rs athul-rs changed the title feat/app-deployment-plugin ( TO-BE-MERGED Later) feat/app-deployment-plugin Jun 12, 2024
Copy link
Contributor

@hari-kuriakose hari-kuriakose left a comment

Choose a reason for hiding this comment

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

@athul-rs LGTM overall.

Added few minor comments though.

Copy link

Copy link
Contributor

filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_logs}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_client\_init}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{worker/src/unstract/worker/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_run\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#23d18b}{\tt{9}}$$

@hari-kuriakose hari-kuriakose merged commit b3afbcc into main Jun 20, 2024
5 checks passed
@hari-kuriakose hari-kuriakose deleted the fix/app-deployment-plugin branch June 20, 2024 06:36
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.

4 participants