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

Setup local proxy via nginx for local development #58

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

timlevett
Copy link
Contributor

Remove needing to disable security around cross origin by running a nginx proxy_pass via Docker for local running/debugging. I've had good success with this approach in other projects, thought I'd share.

This pull request makes the following changes:

  • Adds local.Dockerfile that is just nginx alpine with added config
  • Adds local-nginx.conf which is the proxy server
  • Adds run-local.sh which builds and deploys the docker image with the volume mount
  • yarn.lock changes, not sure exactly why. Just ran yarn install. /shrug
  • Updated README.md with instruction on running locally with docker

@DarthHater
Copy link
Member

@timlevett based on our changes to how we handle runtime config, you might want to take a gander at those to rework a bit of this. Other than that, this is all great! I'll work with you to merge it at some point soon!

local-nginx.conf Outdated
proxy_pass https://ossindex.sonatype.org/;
}

location /api/2/ {
Copy link
Member

Choose a reason for hiding this comment

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

QUESTION: What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy/pasta, I fix it.

local-nginx.conf Outdated
location = /50x.html {
root /usr/share/nginx/html;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Trivial: Newline

local-nginx.conf Outdated
try_files $uri$args $uri$args/ $uri $uri/ /index.html =404;
}

#error_page 404 /404.html;
Copy link
Member

Choose a reason for hiding this comment

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

We actually take 404s and send them back to index.html (this allows the Angular router to attempt to load URLs that wouldn't typically load like: https://search.maven.org/artifact/org.robolectric/junit/4.0-beta-1/jar (since this wouldn't load the index.html page)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems logical, will do

local.Dockerfile Outdated
@@ -0,0 +1,3 @@
FROM nginx:stable-alpine

COPY local-nginx.conf /etc/nginx/conf.d/default.conf
Copy link
Member

Choose a reason for hiding this comment

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

Trivial: Newline

run-local.sh Outdated
-p 80:80 \
--name mvn-search \
-v `pwd`/dist:/usr/share/nginx/html \
mvn-search:latest
Copy link
Member

Choose a reason for hiding this comment

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

Trivial: Newline

run-local.sh Outdated
docker run \
-p 80:80 \
--name mvn-search \
-v `pwd`/dist:/usr/share/nginx/html \
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to do: -v $(pwd)/dist:/usr/share/nginx/html \ here

As well, I believe docker-compose will allow a relative path for a volume, so you might check that out (as you can see all these values using that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe you can do rel path with volumes. Found moby/moby#4830 but looks like they said nope.

@timlevett
Copy link
Contributor Author

Refactored changes and minor cleanup done in f49768d

@DarthHater let me know if you see anything else. Cheers.

@dawidsawa
Copy link

dawidsawa commented Mar 11, 2019

It works well and allowed me to work on SMO locally without any issue, unlike the method of running central-search described in README.md that I wasn't able to get the actual search to work. Is this something we intent to merge into master anytime soon, @DarthHater ?

@timlevett While this works as advertised, is there anything that can be done to improve performance? Any change I make consistently takes over one minute to be synchronised:

Date: 2019-03-10T08:39:08.924Z
Hash: 3eababb6d2d42f2e4e80
Time: 97628ms
Date: 2019-03-10T09:38:35.740Z
Hash: ee6cbfe21ae608609c3e
Time: 85713ms
Date: 2019-03-10T09:40:39.224Z
Hash: 51eb317ecdac01f4bc68
Time: 78270ms

@timlevett
Copy link
Contributor Author

While this works as advertised, is there anything that can be done to improve performance? Any change I make consistently takes over one minute to be synchronised:

Interesting observation @dawidsawa, I have not experienced this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants