Skip to content

Conversation

@findepi
Copy link
Member

@findepi findepi commented Jun 27, 2024

No description provided.

```bash
git clone https://github.com/apache/datafusion
cd arrow-datafusion
git checkout 12.0.0
Copy link
Member Author

Choose a reason for hiding this comment

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

When i tried to check out 12.0.0. tag, the docker build command failed with some unsatisfied dependencies.

git checkout 12.0.0
cd datafusion
docker build -f datafusion-cli/Dockerfile . --tag datafusion-cli
docker run -it -v $(your_data_location):/data datafusion-cli
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd propose to add --rm here and take out -v $(your_data_location):/data.
this would result in the command being directly copy-pastable into terminal without modifications and also would reduce side effects.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could add a comment like

# Run with data mounted at /data in the container
# docker run --rm -it -v $(your_data_location):/data datafusion-cli

# Run in a container without access to the host filesystem
docker run --rm -it  datafusion-cli

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 though the second command doesn't work

andrewlamb@Andrews-MacBook-Pro-2 datafusion % docker run --rm -it  datafusion-cli
error: Invalid value "/data" for '--data-path <DATA_PATH>': Invalid data directory '/data'

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution @findepi 🙏 📖

I think this PR is an improvement over what is on main and thus will merge it. I liked your proposal about adding --rm -- maybe we can do that as a follow on PR

git checkout 12.0.0
cd datafusion
docker build -f datafusion-cli/Dockerfile . --tag datafusion-cli
docker run -it -v $(your_data_location):/data datafusion-cli
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could add a comment like

# Run with data mounted at /data in the container
# docker run --rm -it -v $(your_data_location):/data datafusion-cli

# Run in a container without access to the host filesystem
docker run --rm -it  datafusion-cli

git checkout 12.0.0
cd datafusion
docker build -f datafusion-cli/Dockerfile . --tag datafusion-cli
docker run -it -v $(your_data_location):/data datafusion-cli
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 though the second command doesn't work

andrewlamb@Andrews-MacBook-Pro-2 datafusion % docker run --rm -it  datafusion-cli
error: Invalid value "/data" for '--data-path <DATA_PATH>': Invalid data directory '/data'

@alamb alamb added the documentation Improvements or additions to documentation label Jun 27, 2024
@alamb alamb merged commit 4d16655 into apache:main Jun 27, 2024
@findepi findepi deleted the findepi/fix-running-in-docker-instructions-b05123 branch June 27, 2024 19:20
@findepi
Copy link
Member Author

findepi commented Jun 27, 2024

@alamb thanks for review and testing this, and the merge!
here is the follow-up as requested: #11148

findepi added a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants