Enabling Dynamic Addition of Images for Dockerization #32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@maazmaqsood Please format the code using black. For black settings, check the |
Nilabhra
requested changes
Jul 26, 2023
There was a problem hiding this comment.
@maazmaqsood Added my initial review :). Things are looking good, just needs a few small changes.
examples/etl_pipeline/etl.py
Outdated
| return outdir | ||
|
|
||
| # @task | ||
| # def transform(data:dict) -> pd.DataFrame: |
There was a problem hiding this comment.
Delete commented function if not needed.
There was a problem hiding this comment.
These were already removed in the later pushes.
examples/etl_pipeline/etl.py
Outdated
|
|
||
|
|
||
| # @task | ||
| # def load(df:pd.DataFrame)-> None: |
There was a problem hiding this comment.
Delete commented function if not needed.
There was a problem hiding this comment.
These were already removed in the later pushes.
Thank you @Nilabhra, code has been updated and pushed. resolving the mentioned issues. |
Nilabhra
approved these changes
Jul 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This pull request introduces a significant enhancement to our existing dockerization process. Previously, PIRlib was limited to dockerizing only one image at a time, specifically miniconda, which is essential for creating the final dockerized image. With this update, we have empowered users to dynamically add another image of their choice, which will be seamlessly integrated into the final dockerized image.
Changes Made:
Dynamic Image Inclusion: We have modified the dockerization script to allow users to specify an additional image they want to include in the final dockerized build. This enables more flexibility and customization for different use cases and applications.
Example pipelines change: We have added a new example pipeline, ETL-pipeline and updated multi-backends example. to demonstrate and test the inclusion of dynamic image to the final image.
Why is this Change Important?
The ability to add another image dynamically in the dockerization process significantly enhances the versatility and usefulness of PIRlib. It allows users to tailor their dockerized environments to specific requirements, ensuring that diverse applications and dependencies can be effectively encapsulated within the final image.
Additional Notes:
I have thoroughly tested the changes, both with the default miniconda image and various supplementary images, ensuring a seamless integration process.
I kindly request a comprehensive review of this pull request to ensure the smooth incorporation of this new feature into PIRlib. Your feedback and suggestions are highly valued, and we are committed to addressing any concerns promptly.