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.

Sign up for GitHub

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

Jump to bottom

DG-2001 Adding a script to setup metastore locally with more ease. #3980

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

DG-2001 Adding a script to setup metastore locally with more ease. #3980

wants to merge 1 commit into from

Conversation

hr2904
Copy link
Collaborator

@hr2904 hr2904 commented Jan 15, 2025

Change description

Adding a script to setup metastore locally with more ease.

Type of change

  • New feature (adds functionality)

Related issues

Fix #1

Checklists

Development

  • Lint rules pass locally
  • Application changes have been tested thoroughly
  • Automated tests covering modified code pass

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

Code review

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached as necessary
  • "Ready for review" label attached and reviewers assigned
  • Changes have been reviewed by at least one other contributor
  • Pull request linked to task tracker where applicable

jnkrmg
jnkrmg requested changes Jan 17, 2025
View reviewed changes
Copy link

@jnkrmg jnkrmg left a comment

Choose a reason for hiding this comment

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

Run ruff on the entire script. It'll format the file better.

setup.py
clone_repo(repo_url, repo_dir)


run_command("git fetch --all", cwd=repo_dir)
Copy link

Choose a reason for hiding this comment

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

Just do an explicit git fetch atlan-v0.6.0. Save memory and time.

krsoninikhil
krsoninikhil reviewed Jan 17, 2025
View reviewed changes
setup.py
Installs lazydocker via Homebrew.
"""
print("[INFO] Installing lazydocker (terminal UI for Docker)...")
brew_install("lazydocker")
Copy link

Choose a reason for hiding this comment

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

If some other docker client is installed, may be skip this installation? I use colima and don't want other conflicting clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krsoninikhil krsoninikhil krsoninikhil left review comments

@jnkrmg jnkrmg jnkrmg requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hr2904 @krsoninikhil @jnkrmg