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

CLI command kedro viz build added #1697

Merged
merged 29 commits into from
Jan 16, 2024
Merged

CLI command kedro viz build added #1697

merged 29 commits into from
Jan 16, 2024

Conversation

jitu5
Copy link
Contributor

@jitu5 jitu5 commented Jan 4, 2024

Description

Resolves #1615. Add kedro viz build to build directory of local Kedro Viz instance with static data.

Development notes

  • Utilized a combination of inheritance and the factory pattern.
  • Introduced a base class called "BaseDeployer" which encapsulates the core deployment logic.
  • Created two specialized child classes: "LocalDeployer" and "S3Deployer:
    • "LocalDeployer" is responsible for copying the build and API folders to the kedro-project folder locally without cloud deployment.
    • "S3Deployer" handles the task of uploading the same content to an S3 bucket.

Adopted this design pattern with the flexibility to easily extend to other cloud platforms like Azure and GCP in the future.

QA notes

  • Install editable kedro viz via pip install -e package
  • cd demo-project
  • Run kedro viz build

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

@jitu5 jitu5 added the Python Pull requests that update Python code label Jan 4, 2024
@jitu5 jitu5 requested a review from ravi-kumar-pilla January 4, 2024 16:24
@jitu5 jitu5 self-assigned this Jan 4, 2024
@jitu5 jitu5 marked this pull request as draft January 5, 2024 11:43
README.md Outdated Show resolved Hide resolved
jitu5 and others added 14 commits January 9, 2024 19:42
Signed-off-by: ravi-kumar-pilla <ravi_kumar_pilla@mckinsey.com>
Signed-off-by: ravi-kumar-pilla <ravi_kumar_pilla@mckinsey.com>
…o-viz into feature/cli-build

# Conflicts:
#	package/kedro_viz/integrations/deployment/base_deployer.py
#	package/tests/test_integrations/test_base_deployer.py
@rashidakanchwala rashidakanchwala marked this pull request as ready for review January 15, 2024 12:20
@rashidakanchwala rashidakanchwala requested review from merelcht, noklam and SajidAlamQB and removed request for yetudada January 15, 2024 12:20
Copy link
Contributor

@SajidAlamQB SajidAlamQB left a comment

Choose a reason for hiding this comment

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

This is awesome work @jitu5! 🌟

The implementation looks great just left a minor comment.

Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

I left some small comments, but generally the implementation looks very neat to me 🙂

package/kedro_viz/integrations/deployment/base_deployer.py Outdated Show resolved Hide resolved
package/kedro_viz/launchers/cli.py Outdated Show resolved Hide resolved
Comment on lines +151 to +155

```bash
kedro viz build
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have some docs explaining how to view this build?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, in another PR we will focus on the build docs

Copy link
Contributor

@noklam noklam left a comment

Choose a reason for hiding this comment

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

Thanks! This is a really nice implementation and very close to what I had in mind. It should be relative straightforward to expand the deploy command and add other targets from here.

Great work!

@jitu5 jitu5 requested a review from NeroOkwa January 16, 2024 10:25
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

I left one minor comment, but otherwise this looks great! 👍 ⭐

return S3Deployer(region, bucket_name)
if platform == "local":
return LocalDeployer()
raise ValueError("Invalid platform specified")
Copy link
Member

Choose a reason for hiding this comment

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

It could be useful to add what the value of "platform" given was, so it's easier to debug. E.g. "Invalid platform bla specified"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, Thanks.

@jitu5 jitu5 merged commit 20dc420 into main Jan 16, 2024
17 checks passed
@jitu5 jitu5 deleted the feature/cli-build branch January 16, 2024 15:38
Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla left a comment

Choose a reason for hiding this comment

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

Awesome work !!

Copy link
Contributor

@NeroOkwa NeroOkwa left a comment

Choose a reason for hiding this comment

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

Amazing !

@jitu5 jitu5 mentioned this pull request Jan 22, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kedro Viz Build
8 participants