Skip to content

Pre-built Database extension #2188

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

Merged
merged 14 commits into from
Dec 6, 2021
Merged

Conversation

abhisbyk
Copy link
Contributor

Support for extending the base image with pre-built database for CI/CD scenarios. Supports Oracle Database 19c onwards.

@abhisbyk abhisbyk self-assigned this Nov 22, 2021
@abhisbyk abhisbyk requested a review from gvenzl as a code owner November 22, 2021 06:38
@abhisbyk abhisbyk marked this pull request as draft November 22, 2021 07:01
@abhisbyk abhisbyk marked this pull request as ready for review November 22, 2021 16:00
@abhisbyk abhisbyk marked this pull request as draft November 22, 2021 17:13
Copy link
Member

@cjbj cjbj left a comment

Choose a reason for hiding this comment

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

Some quick comments from reading some of the text. This is not a review of the feature or how it is coded.


## Advantages

This extended image has an additional layer containing pre-created database on top of the base image. So, when this image is used to spin-up a container, the startup time is really fast. In other words, this extended image saves the database creation time (~ 10-20 mins).
Copy link
Member

Choose a reason for hiding this comment

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

Fix the grammar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sentence seems fine to me. Please elaborate what should be changed.

Copy link
Member

Choose a reason for hiding this comment

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

There is an "a" missing before "pre-created"; however, I think we can probably rephrase that to make it easier to understand for the novice Docker user who doesn't know of image layers, etc.

How about: "This extended image includes an already setup database inside the image itself. Although the image size is larger, the startup time of the container includes only the database startup itself, which makes the container startup duration just a couple of seconds."

Would this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, the rephrased sentence has more clarity. Changed accordingly in the README.

@@ -27,6 +27,7 @@ enable_archivelog: false
## ref: http://kubernetes.io/docs/user-guide/persistent-volumes/
persistence:
## Oracle database data Persistent Volume Storage Class, nfs or block
Copy link
Member

Choose a reason for hiding this comment

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

Database has a capital D in its name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@abhisbyk abhisbyk marked this pull request as ready for review November 24, 2021 10:51
Correction in the command for building the prebuiltdb extension

Co-authored-by: Sankar Periyathambi Neelakandan <[email protected]>
yunus-qureshi
yunus-qureshi previously approved these changes Dec 1, 2021
yunus-qureshi
yunus-qureshi previously approved these changes Dec 2, 2021
Signed-off-by: abhisbyk <[email protected]>
@gvenzl gvenzl merged commit 2c6ff15 into oracle:main Dec 6, 2021
@abhisbyk abhisbyk deleted the AbhiK_pre_built_db branch January 31, 2022 04:11
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.

5 participants