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

docs: add docker instructions #842

Merged
merged 13 commits into from
Apr 2, 2021
Merged

docs: add docker instructions #842

merged 13 commits into from
Apr 2, 2021

Conversation

lionel-nj
Copy link
Contributor

@lionel-nj lionel-nj commented Mar 30, 2021

Summary:

This PR adds documentation to run the validator using a Docker environment.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • [ ] Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • [ ] Include screenshot(s) showing how this pull request works and fixes the issue(s)

@lionel-nj lionel-nj added the documentation Anything related to our documentation label Mar 30, 2021
@lionel-nj lionel-nj requested a review from barbeau March 30, 2021 15:47
@lionel-nj lionel-nj self-assigned this Mar 30, 2021
@lionel-nj lionel-nj linked an issue Mar 30, 2021 that may be closed by this pull request
@lionel-nj
Copy link
Contributor Author

lionel-nj commented Mar 31, 2021

So I actually tried to add a badge for the Docker image workflow (the latest was successfully executed according to https://github.com/MobilityData/gtfs-validator/actions/runs/684309427), but in this PR the badge seems to indicate the contrary.

See: Capture d’écran, le 2021-03-31 à 10 21 43

I tried using the branch or events parameters, but no success. @barbeau @maximearmstrong have you experienced that before? Should I use another mechanism to get this badge in a correct state? Also could this be caused by the fact that I am working on a branch and it would be automatically fixed when this branch is merged to master ?

Copy link
Member

@barbeau barbeau left a comment

Choose a reason for hiding this comment

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

Thanks @lionel-nj! Looks good generally, although I'm having issues with the sharing files part - not sure if that's a problem on my end.

Also could this be caused by the fact that I am working on a branch and it would be automatically fixed when this branch is merged to master ?

Possibly - I would say let's merge it and see if it's still showing as "fail", and open another issue if so.

README.md Outdated Show resolved Hide resolved
### Run it

To run the Docker image in a new container:
`docker run -v /myDirectory:/theContainerDirectory -it ghcr.io/mobilitydata/gtfs-validator:v2.0.0`
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to work for me on Windows 10 :(. I tried:

docker run -v /git-projects/gtfs-validator/output:/output -it ghcr.io/mobilitydata/gtfs-validator:v2.0.0

With this, I can execute the validator and pull GTFS files from a URL, but I can't see the GTFS zip file that's on my desktop in Docker, and I can't see the validator output on my desktop. Any idea why this wouldn't work?

Copy link
Contributor Author

@lionel-nj lionel-nj Mar 31, 2021

Choose a reason for hiding this comment

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

/git-projects/gtfs-validator/output:/output this would only give you access to what's inside /git-projects/gtfs-validator/output on your local machine, from output in the Docker container.

To see what's on your desktop from output (in the container), you would have to replace /git-projects/gtfs-validator/output by the/path/to/your/desktop/.

Copy link
Member

@barbeau barbeau Mar 31, 2021

Choose a reason for hiding this comment

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

The /git-projects/gtfs-validator/output path is on my local computer. And I'm trying to use the folder output within the Docker container.

My understanding is that with:

docker run -v /git-projects/gtfs-validator/output:/output -it ghcr.io/mobilitydata/gtfs-validator:v2.0.0

If I put gtfs.zip in my local computer folder /git-projects/gtfs-validator/output, then within the container I should be able to do:

cd output
ls

...and it should show gtfs.zip within that folder.

Similarly, if I run the validator within the container and write results.json to the container output folder, then I should be able to see that file locally on my computer at /git-projects/gtfs-validator/output.

Is the above correct, or am I understanding this wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the above correct, or am I understanding this wrong?

Correct!

Copy link
Member

Choose a reason for hiding this comment

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

Ah - it looks like on Windows I have to provide the drive letter too - so while the above didn't work, this does:

docker run -v c:/git-projects/gtfs-validator/output:/output -it ghcr.io/mobilitydata/gtfs-validator:v2.0.0

Does it work on your machine with a drive letter too? If so I'd recommend updating the example to include a drive letter so it works out-of-the-box for Windows users.

Copy link
Contributor Author

@lionel-nj lionel-nj Apr 2, 2021

Choose a reason for hiding this comment

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

Does it work on your machine with a drive letter too?

Unfortunately no. Will update the docs as suggested. Thanks @barbeau.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 00a9dea.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@lionel-nj
Copy link
Contributor Author

lionel-nj commented Mar 31, 2021

Thanks @lionel-nj! Looks good generally, although I'm having issues with the sharing files part - not sure if that's a problem on my end.

Investigating this, I just asked @maximearmstrong if he was able to do so.

Also could this be caused by the fact that I am working on a branch and it would be automatically fixed when this branch is merged to master ?

Possibly - I would say let's merge it and see if it's still showing as "fail", and open another issue if so.

👍🏾

Copy link
Member

@barbeau barbeau left a comment

Choose a reason for hiding this comment

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

LGTM!

@lionel-nj lionel-nj merged commit 6fd69c2 into master Apr 2, 2021
@lionel-nj lionel-nj deleted the lionel-nj-patch-1 branch April 2, 2021 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker documentation
2 participants