Feature/media proxy pass#531
Conversation
… be a toggle and not rely on .env files
🦋 Changeset detectedLatest commit: e94da6a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Also, I wrote most of this, but Claude really helped with the "closing brace" detection logic. |
mikeybinns
left a comment
There was a problem hiding this comment.
Not pulled down for testing yet, but found some minor static code changes
| const { projectName } = smashConfig; | ||
| const nginxConfigPath = join( | ||
| homedir(), | ||
| "Library/Application Support/Herd/config/valet/Nginx", |
There was a problem hiding this comment.
This would only work on MacOS, so may not work if we allow windows machines in future, or if a freelancer uses Windows.
Suggest changing to this which supports both (where type is from node:os)
| "Library/Application Support/Herd/config/valet/Nginx", | |
| type() === "Darwin" ? "Library/Application Support/Herd/config/valet/Nginx" : ".config\herd\config\nginx", |
| const httpAuthUsername = process.env.STAGING_HTTP_AUTH_USERNAME; | ||
| const httpAuthPassword = process.env.STAGING_HTTP_AUTH_PASSWORD; |
There was a problem hiding this comment.
Staging auth wont change per dev, so this should be defined in the smash.config.ts instead of the .env
There was a problem hiding this comment.
Agree, but other staging details currently live inside the .env no?
SETUP_INSTALL_AND_BUILD_ONLY=false
WORDPRESS_AUTOMATION_USER="Bot"
WORDPRESS_AUTOMATION_PASSWORD="password"
WORDPRESS_USER=""
WORDPRESS_USER_EMAIL=""
WORDPRESS_PASSWORD=""
STAGING_SSH_USERNAME="site"
STAGING_SSH_HOST="33.333.333.333"
STAGING_SSH_PORT="33333"
STAGING_URL="//site.kinsta.cloud"
MEDIA_DOWNLOAD_MONTHS="-1" # Defaults to -1, which will download all media
MEDIA_DOWNLOAD_PATH="/www/public/current/public/wp-content/uploads"
MEDIA_DOWNLOAD_LOCAL_PATH="./public/wp-content/uploads/" # Where the uploads folder should sit locally
There was a problem hiding this comment.
They do but they shouldn't. There will be a migration to smash config at some point which will involve things being in both places so I think it's better for this new command to use smash config.
| console.log( | ||
| "Run `herd restart` or restart Herd via the UI for the changes to take effect.", | ||
| ); |
There was a problem hiding this comment.
This must always happen so it's probably best to just run it.
| import { join } from "node:path"; | ||
| import { getSmashConfig } from "@atomicsmash/smash-config"; | ||
|
|
||
| export const PROXY_MARKER = "location @uploadsproxy"; |
There was a problem hiding this comment.
This variable is only used in this file, so it shouldn't be exported.
|
|
||
| export const PROXY_MARKER = "location @uploadsproxy"; | ||
|
|
||
| export function buildProxyBlock(stagingUrl: string, httpAuth?: string) { |
There was a problem hiding this comment.
This function is only used in this file, so it shouldn't be exported.
| }`; | ||
| } | ||
|
|
||
| export function addProxyBlock(config: string, stagingUrl: string, httpAuth?: string): string { |
There was a problem hiding this comment.
This function is only used in this file, so it shouldn't be exported.
| ); | ||
| } | ||
|
|
||
| export function removeProxyBlock(config: string): string { |
There was a problem hiding this comment.
This function is only used in this file, so it shouldn't be exported.
| "@atomicsmash/cli": major | ||
| --- | ||
|
|
||
| Added new toggle media proxy function |
There was a problem hiding this comment.
Not sure why there's two changesets here, seems to be duplicate
Media Proxy command added. The ability to pass through the HTTP auth that's enabled by default on our staging sites is essential. Obviously, those creds need to be accessible, so the standard
.envvariables have been updated:STAGING_HTTP_AUTH_USERNAMEandSTAGING_HTTP_AUTH_PASSWORDare new additions.If these variables aren't preset, no Auth header is added to the proxy.