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

Execution context #276

Closed
wants to merge 9 commits into from
Closed

Execution context #276

wants to merge 9 commits into from

Conversation

AydinHassan
Copy link
Member

No description provided.

@@ -145,6 +166,44 @@ private function checkRequest(RequestInterface $request, string $fileName): CgiR
return new Success($request);
}

private function setupEnvironment(ExecutionContext $context, SolutionInterface $solution): void
Copy link
Member Author

Choose a reason for hiding this comment

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

extract this crap somewhere

$content
);
}
// sleep(1);
Copy link
Member Author

Choose a reason for hiding this comment

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

docker tests fucked without this:

Error: "Error response from daemon: invalid mount config for type "bind": bind source path does not exist: /host_mnt/private/var/folders/gg/rgfshctn263cdzq1zhn5y9300000gn/T/php-school/3d745671"

files must take a little while to sync 🤷

];
}

public function composer(string $solutionPath, string $composerCommand, array $composerArgs): Process
{
$composerCache = System::tempDir('composer-cache');
Copy link
Member Author

Choose a reason for hiding this comment

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

ugly af, maybe just create it during bootstrap?

{
while(!file_exists($environment->workingDirectory)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

didn't work

@@ -62,7 +65,7 @@ private function getDefaultEnv(): array
return $env;
}

public function phpCgi(string $solutionPath, array $env, string $content): Process
public function phpCgi(Environment $environment, array $env, string $content): Process
Copy link
Member Author

Choose a reason for hiding this comment

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

@mikeymike processFactory is a bit odd for me, because we have cli & cgi runners as separate entities, but then they both depend on processFactory with specific methods for cli and cgi, so it feels like they are all tied together. Can you think of a signature for the processFactory which would generic?

@@ -38,7 +38,7 @@ public function __construct(string $file)
*/
public static function fromFile(string $file): self
{
return new self(InTempSolutionMapper::mapFile($file));
Copy link
Member Author

Choose a reason for hiding this comment

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

Dropped this InTemp mapper now and we just do it right before executing instead, because this also fucked up docker but I can't remember how now. So I added a addFile method to the context/env which then gets written with the solution. Seems cleaner to do it that way anyway.

@AydinHassan AydinHassan changed the base branch from cloud-runtime to master March 16, 2024 10:48
@AydinHassan AydinHassan reopened this Mar 16, 2024
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.

2 participants