Chris/fix docker compose and url #247
Conversation
| @@ -1,3 +1,5 @@ | |||
| # docker-compose is provided for development purposes only and is not | |||
There was a problem hiding this comment.
This is an extremely good and important warning
| @@ -1,3 +1,5 @@ | |||
| # docker-compose is provided for development purposes only and is not | |||
| # intended for use as a production entropic setup. | |||
There was a problem hiding this comment.
Contact your doctor if symptoms persist for more than three days.
| @@ -29,7 +29,7 @@ class Context { | |||
| if (this._parsedUrl) { | |||
| return this._parsedUrl; | |||
| } | |||
| this._parsedUrl = new URL(this.request.url); | |||
| this._parsedUrl = new URL(this.request.url, 'http://entropic.dev'); | |||
There was a problem hiding this comment.
Hrm, should be EXTERNAL_HOST, yes?
There was a problem hiding this comment.
I opted to go with an (inert) URL here because the host value is never used except to keep new URL() from throwing an exception; whereas this code is used from a lot of different services which may not all need (or specify) EXTERNAL_HOST.
| page: Number(context.url.query.page) || 0, | ||
| status: context.url.query.status, | ||
| page: Number(context.url.searchParams.get('page')) || 0, | ||
| status: context.url.searchParams.get('status'), |
There was a problem hiding this comment.
I am sure there's a Good Reason™ why url.parse had to go away, leaving us with this less-nice API.
There was a problem hiding this comment.
I will (grudgingly) cede that if you wanted to port this entire thing to a service worker based system of some cloudflare-y variety, you'd have to make this change anyway.
| REDIS_URL: redis://redis:6379 | ||
| PORT: 3002 | ||
| PGUSER: postgres | ||
| PGDATABASE: entorpic_dev |
| @@ -52,8 +52,8 @@ | |||
| "lint-fix": "prettier --write '**/*.js'", | |||
| "lint-md": "markdownlint \"**/*.md\" -i \"**/node_modules/**\"", | |||
| "lint-registry": "cd services/registry; npm run lint", | |||
| "postinstall": "for d in cli services/{registry,workers,web,common/boltzmann}; do cd $d; npm i; cd -; done", | |||
| "postinstall": "for d in cli services/{registry,workers,web,storage,common/boltzmann}; do cd $d; npm i; cd -; done", | |||
There was a problem hiding this comment.
Hmm... I don't think this will work on Windows.
There was a problem hiding this comment.
I think it would with git-bash or WSL. There's a lot in this project that I don't think would work with cmd.exe or powershell, I don't know of a good way around that besides maybe adding links to docs on WSL in HACKING.md.
new URL's pickiness about needing a FQDN param if given a partial path.new URLspellscontext.url.query.pageascontext.url.searchParams.get('page')