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

updated Dockerfile so tests would pass and image would build #167

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ WORKDIR /usr/src/databricks-cli

COPY . .

RUN pip install --upgrade pip && \
pip install -r dev-requirements.txt && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a minimal fix here just to also install tox-requirements?

Copy link
Author

Choose a reason for hiding this comment

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

That is certainly the heart of the change -- and almost enough to fix the issue,

You also need to bump the versions on some of the packages in tox-requirements.txt or it will fail due to pytest-django and pyflakes dependencies. Also, you need to add the tabulate and click packages to dev-requirements.txt.

My other edits to Dockerfile were an attempt to minimize the resulting image size, but I didn't get a dramatic savings.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha. I'm a bit concerned about changing the dev-requirements.txt and tox-requirements.txt to make this docker container build work.

I think a minimal fix is to intall tox-requirements.txt and to move up pip install . above the ./lint.sh

Also it may be useful to add a .dockerignore file containing

**/__pycache__
**/*.pyc

so we don't run into https://stackoverflow.com/questions/44067609/getting-error-importmismatcherror-while-running-py-test

Choose a reason for hiding this comment

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

with @andrewmchen proposals (add tox-requirements to pip install and moving ./lint.sh) works without problem.

thanks

RUN pip install --upgrade --no-cache-dir pip && \
pip install --no-cache-dir \
-r dev-requirements.txt \
-r tox-requirements.txt && \
pip list && \
./lint.sh && \
pip install . && \
pytest tests
pip install --no-cache-dir . && \
pytest tests && \
pip uninstall -y \
-r tox-requirements.txt

ENTRYPOINT [ "databricks" ]
4 changes: 2 additions & 2 deletions databricks_cli/workspace/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ def import_workspace_dir(self, source_path, target_path, overwrite, exclude_hidd
click.echo(('{} does not have a valid extension of {}. Skip this file and ' +
'continue.').format(cur_src, extensions))

def export_workspace_dir(self, source_path, target_path, overwrite):
def export_workspace_dir(self, source_path, target_path, format=WorkspaceFormat.SOURCE, overwrite):
if os.path.isfile(target_path):
click.echo('{} exists as a file. Skipping this subtree {}'
.format(target_path, source_path))
Expand All @@ -167,7 +167,7 @@ def export_workspace_dir(self, source_path, target_path, overwrite):
elif obj.is_notebook:
cur_dst = cur_dst + WorkspaceLanguage.to_extension(obj.language)
try:
self.export_workspace(cur_src, cur_dst, WorkspaceFormat.SOURCE, overwrite)
self.export_workspace(cur_src, cur_dst, format, overwrite)
click.echo('{} -> {}'.format(cur_src, cur_dst))
except LocalFileExistsException:
click.echo('{} already exists locally as {}. Skip.'.format(cur_src, cur_dst))
Expand Down
6 changes: 4 additions & 2 deletions databricks_cli/workspace/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,22 +147,24 @@ def delete_cli(api_client, workspace_path, recursive):
@click.argument('source_path')
@click.argument('target_path')
@click.option('--overwrite', '-o', is_flag=True, default=False)
@click.option('--format', '-f', default=WorkspaceFormat.SOURCE, type=FormatClickType())
@debug_option
@profile_option
@eat_exceptions
@provide_api_client
def export_dir_cli(api_client, source_path, target_path, overwrite):
def export_dir_cli(api_client, source_path, target_path, format, overwrite):
"""
Recursively exports a directory from the Databricks workspace.

Only directories and notebooks are exported. Notebooks are always exported in the SOURCE
format. Notebooks will also have the extension of .scala, .py, .sql, or .r appended
depending on the language type.
ADDED format -- jeffreybreen
"""
workspace_api = WorkspaceApi(api_client)
assert workspace_api.get_status(source_path).is_dir, 'The source path must be a directory. {}' \
.format(source_path)
workspace_api.export_workspace_dir(source_path, target_path, overwrite)
workspace_api.export_workspace_dir(source_path, target_path, format, overwrite)


@click.command(context_settings=CONTEXT_SETTINGS,
Expand Down
4 changes: 3 additions & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
tox==2.9.1
tox==3.2.1
click==6.7
tabulate==0.8.2
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
'Intended Audience :: System Administrators',
'Programming Language :: Python :: 2.7',
'Programming Language :: Python :: 3.6',
'License :: OSI Approved :: Apache Software License'
],
keywords='databricks cli',
url='https://github.com/databricks/databricks-cli'
Expand Down
12 changes: 6 additions & 6 deletions tox-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# Test reqs
prospector[with_pyroma]==0.12.7
pylint==1.8.2
pep8-naming==0.5.0
pytest==3.2.1
prospector[with_pyroma]==1.1.1
pylint==1.9.3
pep8-naming==0.7.0
pytest==3.7.1
mock==2.0.0
decorator==4.2.1
rstcheck==3.2
decorator==4.3.0
rstcheck==3.3
pytest-cov
codecov