Skip to content

Commit

Permalink
Merge pull request #122 from keboola/miro-fix-om
Browse files Browse the repository at this point in the history
PST-2030: fix (OutputManifest): don't overwrite source with destination
  • Loading branch information
MiroCillik authored Oct 25, 2024
2 parents 74b834f + ef6c031 commit d09a2cc
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 21 deletions.
24 changes: 18 additions & 6 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@ env:

DOCKERHUB_USER: '${{ secrets.DOCKERHUB_USER }}'
DOCKERHUB_TOKEN: '${{ secrets.DOCKERHUB_TOKEN }}'
KBC_STORAGE_TOKEN: '${{ secrets.KBC_STORAGE_TOKEN }}'
KBC_TEST_PROJECT_URL: ''
KBC_TEST_PROJECT_CONFIGS: ''
KBC_TEST_PROJECT_URL: 'https://connection.keboola.com/admin/projects/9317'
KBC_TEST_PROJECT_CONFIGS: '1192751064'
GCP_KBC_TEST_PROJECT_URL: 'https://connection.europe-west3.gcp.keboola.com/admin/projects/55'
GCP_KBC_TEST_PROJECT_CONFIGS: '49197'

jobs:
build:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -210,13 +212,23 @@ jobs:
steps:
-
name: 'Run KBC test jobs'
if: 'env.KBC_STORAGE_TOKEN && env.KBC_TEST_PROJECT_CONFIGS'
if: 'env.KBC_TOKEN && env.KBC_TEST_PROJECT_CONFIGS'
uses: keboola/action-run-configs-parallel@master
with:
token: '${{ env.KBC_STORAGE_TOKEN }}'
token: '${{ env.KBC_TOKEN }}'
componentId: '${{ env.KBC_DEVELOPERPORTAL_APP }}'
tag: '${{ needs.build.outputs.app_image_tag }}-1.5.0'
tag: '${{ needs.build.outputs.app_image_tag }}-1.8.6'
configs: '${{ env.KBC_TEST_PROJECT_CONFIGS }}'
-
name: 'Run KBC test jobs (GCP BQ)'
if: 'env.GCP_KBC_TOKEN && env.GCP_KBC_TEST_PROJECT_CONFIGS'
uses: keboola/action-run-configs-parallel@master
with:
host: '${{ env.GCP_KBC_URL }}'
token: '${{ env.GCP_KBC_TOKEN }}'
componentId: 'keboola.dbt-transformation-local-bigquery'
tag: '${{ needs.build.outputs.app_image_tag }}-1.8.6'
configs: '${{ env.GCP_KBC_TEST_PROJECT_CONFIGS }}'
deploy:
needs:
- build
Expand Down
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/env
/vendor
/.idea
.env
/.idea
/.phpunit.result.cache
17 changes: 14 additions & 3 deletions src/FileDumper/OutputManifest/OutputManifest.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected function processTableDefinition(
array $configuredOutputTables,
): void {
$tableName = $tableDef->getTableName();
$destinationTableName = $tableName;
$destinationTableName = null;
$configuredPrimaryKeys = [];
if ($configuredOutputTables !== []) {
foreach ($configuredOutputTables as $configuredOutputTable) {
Expand All @@ -99,7 +99,7 @@ protected function processTableDefinition(
}
}

$realTableName = $this->getRealTableName($destinationTableName);
$realTableName = $this->getRealTableName($tableName);
$dbtColumnsMetadata = $dbtMetadata[$tableName]['column_metadata'] ?? [];

if ($configuredPrimaryKeys !== []) {
Expand All @@ -111,7 +111,14 @@ protected function processTableDefinition(
$columnsMetadata = $this->getColumnsMetadata($tableDef, $dbtColumnsMetadata);
$tableMetadata = $this->getTableMetadata($tableName, $realTableName, $dbtMetadata);

$this->createAndWriteTableManifest($tableDef, $realTableName, $tableMetadata, $columnsMetadata, $dbtPrimaryKey);
$this->createAndWriteTableManifest(
$tableDef,
$realTableName,
$tableMetadata,
$columnsMetadata,
$dbtPrimaryKey,
$destinationTableName,
);
}

/**
Expand Down Expand Up @@ -160,6 +167,7 @@ protected function createAndWriteTableManifest(
array $tableMetadata,
object $columnsMetadata,
array $dbtPrimaryKey,
?string $destination = null,
): void {
$tableManifestOptions = new ManifestOptions();
$primaryKeys = self::getPrimaryKeyColumnNames(
Expand Down Expand Up @@ -205,6 +213,9 @@ protected function createAndWriteTableManifest(
}
$tableManifestOptions->setSchema($schema);
$tableManifestOptions->setTableMetadata($tableMetadataKeyValue);
if ($destination) {
$tableManifestOptions->setDestination($destination);
}

$this->manifestManager->writeTableManifest($realTableName, $tableManifestOptions, $this->legacyFormat);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1 of 2 ERROR STALE freshness of in.c-test-bucket.test .......................... [ERROR STALE in %ss]
1 of 1 ERROR STALE freshness of in.c-test-bucket.test .......................... [ERROR STALE in %ss]
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
"warn_after": {"active": true, "count": 1, "period": "minute"},
"error_after": {"active": true, "count": 2, "period": "minute"}
}
},
"storage": {
"input": {
"tables": [
{
"source": "in.c-test-bucket.test"
}
]
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ Executing command "dbt source freshness"
Running with dbt=%A
Found 2 models%S
%AConcurrency: 1 threads (target='kbc_prod')%A
1 of 2 START freshness of in.c-test-bucket.test %s [RUN]%A
%aDone.
1 of 1 START freshness of in.c-test-bucket.test %s [RUN]%A
%ADone.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@
"freshness": {
"warn_after": {"active": true, "count": 1, "period": "day"}
}
},
"storage": {
"input": {
"tables": [
{
"source": "in.c-test-bucket.test"
}
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
{
"destination": "out.c-test.fct_model",
"metadata": [
{
"key": "KBC.description",
"value": ""
},
{
"key": "KBC.name",
"value": "out.c-test.fct_model"
"value": "fct_model"
},
{
"key": "KBC.datatype.backend",
Expand Down Expand Up @@ -96,4 +97,4 @@
}
]
}
}
}
6 changes: 4 additions & 2 deletions tests/phpunit/FileDumper/OutputManifestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -777,13 +777,14 @@ public function testDumpJsonWithOutputTables(): void
],
]);

$tableManifestPath1 = $this->dataDir . '/out/tables/OUT.TEST.BEERS_WITH_BREWERIES.manifest';
$tableManifestPath1 = $this->dataDir . '/out/tables/BEERS_WITH_BREWERIES.manifest';
self::assertFileExists($tableManifestPath1);

$tableManifestPath2 = $this->dataDir . '/out/tables/beers.manifest';
self::assertFileDoesNotExist($tableManifestPath2);

/** @var array{
* 'destination': string,
* 'primary_key': array<string>,
* 'columns': array<string>,
* 'metadata': array<int, array<string, string>>,
Expand All @@ -792,6 +793,7 @@ public function testDumpJsonWithOutputTables(): void
*/
$manifest1 = (array) json_decode((string) file_get_contents($tableManifestPath1), true);

self::assertSame('out.test.beers_with_breweries', $manifest1['destination']);
self::assertEquals([
'brewery_id',
'beer_id',
Expand All @@ -816,7 +818,7 @@ public function testDumpJsonWithOutputTables(): void
],
[
'key' => 'KBC.name',
'value' => 'OUT.TEST.BEERS_WITH_BREWERIES',
'value' => 'BEERS_WITH_BREWERIES',
],
[
'key' => 'KBC.datatype.backend',
Expand Down
9 changes: 5 additions & 4 deletions tests/phpunit/Service/DbtServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -165,24 +165,25 @@ public function testDbtCompile(string $backend, string $firstSql, string $second

$stringsToFind = [
'/Starting full parse./',
'/Found 2 models, 2 (data )?tests/',
'/Found 2 models/',
];

$foundedCount = 0;
$foundCount = 0;
foreach ($parsedOutput as $line) {
foreach ($stringsToFind as $stringToFind) {
if (preg_match($stringToFind, $line) === 1) {
$foundedCount++;
$foundCount++;
};
}
}
if ($foundedCount !== count($stringsToFind)) {
if ($foundCount !== count($stringsToFind)) {
self::fail('Not all strings were found in output');
}

$compiledSql = DbtCompileHelper::getCompiledSqlFilesContent($this->getProjectPath() . '/target');

$keys = array_keys($compiledSql);

self::assertContains('source_not_null_in.c-test-bucket_test__id_.sql', $keys);
self::assertContains('source_unique_in.c-test-bucket_test__id_.sql', $keys);
self::assertContains('fct_model.sql', $keys);
Expand Down

0 comments on commit d09a2cc

Please sign in to comment.