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

Updates to the TPS pipeline to render tiles end-to-end #9

Merged
merged 35 commits into from Aug 17, 2018

Conversation

rmarianski
Copy link
Member

These are the updates that I've been tracking locally up until the step to make rawr tiles.

Robert Marianski added 9 commits August 1, 2018 14:44
Eliminate the need for GOPATH. The go binaries are expected to already
be installed as pre-requisites.
* add docker (needed for building images)
* add user to docker group
* add tz-missing-meta-tiles-write command
* GOPATH env var is needed to find location of binaries
@@ -102,7 +102,7 @@ def vpc_of_sg(sg_id):
'missing-meta-tiles-write'):
config = {
'logging': {
'config': 'logging.conf.sample'
'config': 'batch-setup/logging.conf.sample'
Copy link
Member

Choose a reason for hiding this comment

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

Should we have the full path to the logging config (i.e: os.path.join(os.path.dirname(os.path.realpath(__file__)), 'logging.conf.sample') or similar), so that it doesn't matter what the working directory is?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, I'm always running all commands from the tileops root anyway because I'm not sure if there would be other problems, but we certainly don't lose anything by using the full path and it's easy to do. db0cecb

@@ -24,6 +26,7 @@ for repo in raw_tiles tilequeue vector-datasource tileops; do
done
cat > /usr/local/etc/planet-env.sh << eof
#!/bin/bash
export GOPATH=/usr/local
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it would stomp on other stuff in /usr/local/. Should we put it in /usr/local/gopath and add /usr/local/gopath/bin to $PATH in /etc/profile?

Copy link
Member

Choose a reason for hiding this comment

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

On second thoughts, it looks like the changes to docker/missing-meta-tiles-write/Makefile mean that we don't need $GOPATH at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems like it would stomp on other stuff in /usr/local/

Is there a problem with that though?

I've been trying to move away from having any go build requirements on the instance itself. I put the binaries themselves onto s3, and we're just pulling those down on startup. I wasn't sure if there was anything else that needed it, so just setting GOPATH seemed like a faster way to sidestep the problem.

There's code in the python that expects GOPATH to be set, and from what I can tell it's just to find the commands in the first place. Should we just explicitly set that instead? ie take in the path to where those commands are via a command line arg, and not set GOPATH at all?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can make it optional? When I've been running this locally, $GOPATH/bin was never in my $PATH (and I get kinda paranoid about not having random stuff in my $PATH), so $GOPATH seemed like a sensible way to do it. Do you think we could change the script so that it looks for the binary in $GOPATH if it's set, in $PATH if not, and errors out if it doesn't find the binary in either?

Copy link
Member Author

Choose a reason for hiding this comment

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

With my pedantic hat on, searching for commands in $GOPATH at runtime isn't ideal. It's just meant to have meaning at compile time. $PATH really does seem like the right place for it. If we don't want to have these commands in $PATH, we could just pass in the full paths to them via config or command line, or just put all of these in a separate base path these we send in via an arg.

I took the hat off, and made the change in 18315b1 :)

Robert Marianski and others added 18 commits August 2, 2018 11:37
…submitted. This can be really useful for shortening the compile-run-debug cycle.
…ge, and whether the list printed is the missing tiles or the present tiles.
…edly bouncing an environment. Added extra wait for job queue deletion, as this would occasionally take long enough to cause an error with compute environment deletion.
@zerebubuth zerebubuth changed the title Updates up until making rawr tiles Updates to the TPS pipeline to render tiles end-to-end Aug 17, 2018
@zerebubuth
Copy link
Member

There's still a bunch of stuff that needs adding to this, but it's become a monster PR and it would be more readable to make smaller PRs for the extra changes.

@zerebubuth zerebubuth merged commit 19a2728 into master Aug 17, 2018
@zerebubuth zerebubuth deleted the tps-updates branch August 17, 2018 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants