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

fix: Updated run-platform to create schema for new users #820

Merged
merged 4 commits into from
Oct 28, 2024

Conversation

chandrasekharan-zipstack
Copy link
Contributor

@chandrasekharan-zipstack chandrasekharan-zipstack commented Oct 25, 2024

What

  • Fix in run-platform.sh to create DB schema
  • Fix in migrate-to-v2.sh to allow talking to DB / redis in containers
  • Updated ENTRYPOINT to CMD in backend's Dockerfile, to allow invoking the necessary schema creation command
  • Renamed and moved the schema creation / drop commands to utils app

Why

  • Schema creation has to be done explicitly once as part of the v2 changes, previously this was done by django-tenant and the public schema would be present by default
  • As a result new users faced this issue

How

  • For first time users, schema creation handled with command run on backend container
  • For existing users, they will have to read the release notes and manually run backend/migrating/v2/migrate-to-v2.sh to perform the schema creation and subsequent migration

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, since it touches the backend Dockerfile and some scripts used by OSS users - it is possible

Database Migrations

  • Might run when these scripts are invoked

Related Issues or PRs

Notes on Testing

  • Removed /etc/hosts entries to mimic the average OSS user to reproduce the same issues and worked on unblocking myself

Screenshots Old

  • To be used by new users, ran with trace option
    image

  • Migration script (to be used by existing users)
    image

Screenshots after addressing comments

  • Schema creation handled through entrypoint script
    image

Checklist

I have read and understood the Contribution Guidelines.

run-platform.sh Outdated Show resolved Hide resolved
@hari-kuriakose
Copy link
Contributor

@chandrasekharan-zipstack Let's also move scripts/ to docker/scripts/ along with this.

run-platform.sh Outdated Show resolved Hide resolved
run-platform.sh Outdated Show resolved Hide resolved
@chandrasekharan-zipstack
Copy link
Contributor Author

@chandrasekharan-zipstack Let's also move scripts/ to docker/scripts/ along with this.

@hari-kuriakose I've addressed it in this PR - can we let these be separate changes? Will resolve any conflicts accordingly

…schema creation commands, minor run-platform.sh changes
@jaseemjaskp jaseemjaskp self-requested a review October 26, 2024 12:44
Copy link
Contributor

@jaseemjaskp jaseemjaskp 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

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}}$$

Copy link

sonarcloud bot commented Oct 28, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarCloud

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.

@hari-kuriakose
Copy link
Contributor

Can take care of the Django management commands security hotspots separately.

@hari-kuriakose hari-kuriakose merged commit 125a930 into main Oct 28, 2024
4 of 5 checks passed
@hari-kuriakose hari-kuriakose deleted the fix/schema-creation-for-oss-users branch October 28, 2024 11:47
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.

3 participants