Skip to content

Commit

Permalink
refactor: PST-2117 clearer naming of the enum for DWH connection type…
Browse files Browse the repository at this point in the history
…, move DwhConnectionType to the constructor of DbtService
  • Loading branch information
Jiří Novák committed Oct 21, 2024
1 parent b1b517b commit 1cb157b
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 53 deletions.
12 changes: 6 additions & 6 deletions src/Component.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use DbtTransformation\Configuration\SyncAction\DbtDocsDefinition;
use DbtTransformation\Configuration\SyncAction\DbtRunResultsDefinition;
use DbtTransformation\Configuration\SyncAction\GitRepositoryDefinition;
use DbtTransformation\DwhProvider\DwhLocationEnum;
use DbtTransformation\DwhProvider\DwhConnectionTypeEnum;
use DbtTransformation\DwhProvider\DwhProviderFactory;
use DbtTransformation\Exception\ArtifactNotFoundException;
use DbtTransformation\FileDumper\OutputManifest\DbtManifestParser;
Expand Down Expand Up @@ -211,18 +211,18 @@ protected function logExecutedSqls(): void
/**
* @throws \Keboola\Component\UserException
*/
protected function executeStep(string $step, DwhLocationEnum $dwhLocation): void
protected function executeStep(string $step, DwhConnectionTypeEnum $dwhLocation): void
{
$this->getLogger()->info(sprintf('Executing command "%s"', $step));
$dbtService = new DbtService($this->projectPath);
$dbtService = new DbtService($this->projectPath, $dwhLocation);
if ($step === DbtService::COMMAND_DEPS) {
//some deps could be installed from git, so retry for "shallow file has changed" is needed
/** @var string $output */
$output = $this->gitRetryProxy->call(function () use ($dbtService, $step, $dwhLocation): string {
return $dbtService->runCommand($step, $dwhLocation);
$output = $this->gitRetryProxy->call(function () use ($dbtService, $step): string {
return $dbtService->runCommand($step);
});
} else {
$output = $dbtService->runCommand($step, $dwhLocation);
$output = $dbtService->runCommand($step);
}

foreach (ParseDbtOutputHelper::getMessagesFromOutput($output) as $log) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace DbtTransformation\DwhProvider;

enum DwhLocationEnum
enum DwhConnectionTypeEnum
{
case LOCAL;
case REMOTE;
Expand Down
2 changes: 1 addition & 1 deletion src/DwhProvider/DwhProviderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ interface DwhProviderInterface
{
public function createDbtYamlFiles(): void;

public function getDwhLocation(): DwhLocationEnum;
public function getDwhLocation(): DwhConnectionTypeEnum;
}
4 changes: 2 additions & 2 deletions src/DwhProvider/LocalBigQueryProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ public function __destruct()
$this->temp->remove();
}

public function getDwhLocation(): DwhLocationEnum
public function getDwhLocation(): DwhConnectionTypeEnum
{
return DwhLocationEnum::LOCAL;
return DwhConnectionTypeEnum::LOCAL;
}
}
4 changes: 2 additions & 2 deletions src/DwhProvider/LocalSnowflakeProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ public static function getDbtParams(): array
];
}

public function getDwhLocation(): DwhLocationEnum
public function getDwhLocation(): DwhConnectionTypeEnum
{
return DwhLocationEnum::LOCAL;
return DwhConnectionTypeEnum::LOCAL;
}
}
4 changes: 2 additions & 2 deletions src/DwhProvider/RemoteProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ abstract public static function getRequiredConnectionParams(): array;
*/
abstract public static function getDbtParams(): array;

public function getDwhLocation(): DwhLocationEnum
public function getDwhLocation(): DwhConnectionTypeEnum
{
return DwhLocationEnum::REMOTE;
return DwhConnectionTypeEnum::REMOTE;
}
}
31 changes: 15 additions & 16 deletions src/Service/DbtService.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace DbtTransformation\Service;

use DbtTransformation\DwhProvider\DwhLocationEnum;
use DbtTransformation\DwhProvider\DwhConnectionTypeEnum;
use DbtTransformation\Helper\ParseDbtOutputHelper;
use Keboola\Component\UserException;
use Symfony\Component\Console\Input\StringInput;
Expand All @@ -31,20 +31,19 @@ class DbtService
public const COMMAND_RUN = 'dbt run';
public const COMMAND_DEPS = 'dbt deps';

private string $projectPath;

public function __construct(string $projectPath)
{
$this->projectPath = $projectPath;
public function __construct(
private readonly string $projectPath,
private readonly DwhConnectionTypeEnum $dwhConnectionType,
) {
}

/**
* @throws \Keboola\Component\UserException
*/
public function runCommand(string $command, DwhLocationEnum $dwhLocation, string $target = 'kbc_prod'): string
public function runCommand(string $command, string $target = 'kbc_prod'): string
{
try {
$command = $this->prepareCommand($command, $dwhLocation, $target);
$command = $this->prepareCommand($command, $target);
$process = new Process($command, $this->projectPath, getenv(), null, null);
$process->mustRun();
return $process->getOutput();
Expand All @@ -65,14 +64,14 @@ public function runCommand(string $command, DwhLocationEnum $dwhLocation, string
* @return array<int, string>
* @throws \Keboola\Component\UserException
*/
protected function prepareCommand(string $command, DwhLocationEnum $dwhLocation, string $target): array
protected function prepareCommand(string $command, string $target): array
{
return [
'dbt',
'--log-format',
'json',
'--no-use-colors',
...$this->getCommandWithoutDbt($command, $dwhLocation),
...$this->getCommandWithoutDbt($command),
'-t',
$target,
'--profiles-dir',
Expand All @@ -84,12 +83,12 @@ protected function prepareCommand(string $command, DwhLocationEnum $dwhLocation,
* @return array<string>
* @throws \Keboola\Component\UserException
*/
protected function getCommandWithoutDbt(string $commandString, DwhLocationEnum $dwhLocation): array
protected function getCommandWithoutDbt(string $commandString): array
{
$stringInput = new StringInput($commandString);
$command = $stringInput->getRawTokens(true);
foreach ($command as $commandPart) {
$foundOption = $this->findDisallowedOption($commandPart, $dwhLocation);
$foundOption = $this->findDisallowedOption($commandPart);
if ($foundOption !== null) {
throw new UserException("You cannot override option {$foundOption} in your dbt command. " .
'Please remove it.');
Expand All @@ -99,11 +98,11 @@ protected function getCommandWithoutDbt(string $commandString, DwhLocationEnum $
return $command;
}

private function findDisallowedOption(string $commandPart, DwhLocationEnum $dwhLocation): ?string
private function findDisallowedOption(string $commandPart): ?string
{
$disallowedOptions = match ($dwhLocation) {
DwhLocationEnum::LOCAL => self::DISALLOWED_OPTIONS_LOCAL_DWH,
DwhLocationEnum::REMOTE => self::DISALLOWED_OPTIONS_REMOTE_DWH,
$disallowedOptions = match ($this->dwhConnectionType) {
DwhConnectionTypeEnum::LOCAL => self::DISALLOWED_OPTIONS_LOCAL_DWH,
DwhConnectionTypeEnum::REMOTE => self::DISALLOWED_OPTIONS_REMOTE_DWH,
};

foreach ($disallowedOptions as $option) {
Expand Down
48 changes: 25 additions & 23 deletions tests/phpunit/Service/DbtServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use ColinODell\PsrTestLogger\TestLogger;
use DbtTransformation\Config;
use DbtTransformation\Configuration\ConfigDefinition;
use DbtTransformation\DwhProvider\DwhLocationEnum;
use DbtTransformation\DwhProvider\DwhConnectionTypeEnum;
use DbtTransformation\DwhProvider\DwhProviderFactory;
use DbtTransformation\Helper\DbtCompileHelper;
use DbtTransformation\Helper\ParseDbtOutputHelper;
Expand All @@ -32,7 +32,7 @@ public function setUp(): void
$this->gitRepositoryService = new GitRepositoryService($this->dataDir);
$logger = new TestLogger();

$this->dbtService = new DbtService($this->getProjectPath());
$this->dbtService = new DbtService($this->getProjectPath(), DwhConnectionTypeEnum::LOCAL);
$this->dwhProviderFactory = new DwhProviderFactory($logger);
}

Expand Down Expand Up @@ -71,10 +71,11 @@ private function getConfig(string $backend, string $executeStep): Config
/**
* @dataProvider validDbtOptionsProvider
*/
public function testValidDbtOptions(string $command, DwhLocationEnum $dwhLocation): void
public function testValidDbtOptions(string $command, DwhConnectionTypeEnum $dwhConnectionType): void
{
$dbtService = new DbtService($this->getProjectPath(), $dwhConnectionType);
try {
$this->dbtService->runCommand($command, $dwhLocation);
$dbtService->runCommand($command);
} catch (UserException $e) {
$this->fail('Command should not fail with valid options: ' . $e->getMessage());
} catch (Throwable) {
Expand All @@ -88,61 +89,62 @@ public function testValidDbtOptions(string $command, DwhLocationEnum $dwhLocatio
*/
public function testInvalidDbtOptions(
string $command,
DwhLocationEnum $dwhLocation,
DwhConnectionTypeEnum $dwhConnectionType,
string $expectedExceptionMessage,
): void {
$this->expectException(UserException::class);
$this->expectExceptionMessage($expectedExceptionMessage);

$this->dbtService->runCommand($command, $dwhLocation);
$dbtService = new DbtService($this->getProjectPath(), $dwhConnectionType);
$dbtService->runCommand($command);
}

/**
* @return Generator<string, array{0: string, 1: DwhLocationEnum}>
* @return Generator<string, array{0: string, 1: DwhConnectionTypeEnum}>
*/
public function validDbtOptionsProvider(): Generator
{
yield 'valid option resource-type local dwh' => ['dbt ls --resource-type model', DwhLocationEnum::LOCAL];
yield 'valid option models local dwh' => ['dbt run --models my_model', DwhLocationEnum::LOCAL];
yield 'valid option resource-type remote dwh' => ['dbt ls --resource-type model', DwhLocationEnum::REMOTE];
yield 'valid option models remote dwh' => ['dbt run --models my_model', DwhLocationEnum::REMOTE];
yield 'valid option target remote dwh' => ['dbt run --target dev', DwhLocationEnum::REMOTE];
yield 'valid option shorthand target remote dwh' => ['dbt run -t dev', DwhLocationEnum::REMOTE];
yield 'valid option resource-type local' => ['dbt ls --resource-type model', DwhConnectionTypeEnum::LOCAL];
yield 'valid option models local' => ['dbt run --models my_model', DwhConnectionTypeEnum::LOCAL];
yield 'valid option resource-type remote' => ['dbt ls --resource-type model', DwhConnectionTypeEnum::REMOTE];
yield 'valid option models remote' => ['dbt run --models my_model', DwhConnectionTypeEnum::REMOTE];
yield 'valid option target remote' => ['dbt run --target dev', DwhConnectionTypeEnum::REMOTE];
yield 'valid option shorthand target remote' => ['dbt run -t dev', DwhConnectionTypeEnum::REMOTE];
}

/**
* @return Generator<string, array{0: string, 1: DwhLocationEnum, 2: string}>
* @return Generator<string, array{0: string, 1: DwhConnectionTypeEnum, 2: string}>
*/
public function invalidDbtOptionsProvider(): Generator
{
yield 'invalid option profiles-dir local dwh' => [
'dbt run --profiles-dir dir',
DwhLocationEnum::LOCAL,
DwhConnectionTypeEnum::LOCAL,
'You cannot override option --profiles-dir in your dbt command. Please remove it.',
];
yield 'invalid option log-format local dwh' => [
'dbt run --log-format json',
DwhLocationEnum::LOCAL,
DwhConnectionTypeEnum::LOCAL,
'You cannot override option --log-format in your dbt command. Please remove it.',
];
yield 'invalid option target local dwh' => [
'dbt run --target dev',
DwhLocationEnum::LOCAL,
DwhConnectionTypeEnum::LOCAL,
'You cannot override option --target in your dbt command. Please remove it.',
];
yield 'invalid option shorthand target local dwh' => [
'dbt run -t dev',
DwhLocationEnum::LOCAL,
DwhConnectionTypeEnum::LOCAL,
'You cannot override option -t in your dbt command. Please remove it.',
];
yield 'invalid option profiles-dir remote dwh' => [
'dbt run --profiles-dir dir',
DwhLocationEnum::REMOTE,
DwhConnectionTypeEnum::REMOTE,
'You cannot override option --profiles-dir in your dbt command. Please remove it.',
];
yield 'invalid option log-format remote dwh' => [
'dbt run --log-format json',
DwhLocationEnum::REMOTE,
DwhConnectionTypeEnum::REMOTE,
'You cannot override option --log-format in your dbt command. Please remove it.',
];
}
Expand All @@ -161,7 +163,7 @@ public function testDbtDebug(string $backend): void
$provider = $this->dwhProviderFactory->getProvider($config, $this->getProjectPath());
$provider->createDbtYamlFiles();

$output = $this->dbtService->runCommand(DbtService::COMMAND_DEBUG, DwhLocationEnum::LOCAL);
$output = $this->dbtService->runCommand(DbtService::COMMAND_DEBUG);

self::assertStringContainsString('profiles.yml file', $output);
self::assertStringContainsString('dbt_project.yml file', $output);
Expand All @@ -182,7 +184,7 @@ public function testDbtCompile(string $backend, string $firstSql, string $second
$this->gitRepositoryService->clone('https://github.com/keboola/dbt-test-project-public.git');
$provider = $this->dwhProviderFactory->getProvider($config, $this->getProjectPath());
$provider->createDbtYamlFiles();
$output = $this->dbtService->runCommand(DbtService::COMMAND_COMPILE, DwhLocationEnum::LOCAL);
$output = $this->dbtService->runCommand(DbtService::COMMAND_COMPILE);
$parsedOutput = iterator_to_array(ParseDbtOutputHelper::getMessagesFromOutput($output));

$stringsToFind = [
Expand Down Expand Up @@ -235,7 +237,7 @@ public function testDbtRunWithVars(): void
$provider = $this->dwhProviderFactory->getProvider($config, $this->getProjectPath());
$provider->createDbtYamlFiles();

$output = $this->dbtService->runCommand($command, DwhLocationEnum::LOCAL);
$output = $this->dbtService->runCommand($command);

self::assertStringContainsString('"vars": "{\'var1\': \'value1\', \'var2\': \'value2\'}', $output);
}
Expand Down

0 comments on commit 1cb157b

Please sign in to comment.