Fix sandbox-verification workflow YAML parsing, Python argument, and generator type errors#45
Conversation
…parsing error Lines 183-184 and 188-189 had Python code at column 1 (zero indentation) inside a `run: |` block scalar, which broke YAML parsing. GitHub showed the workflow run as failed with 0 jobs because it couldn't parse the file. Fixed by inlining the Python code on single lines using semicolons. Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/4c9208a0-04e7-4727-b061-0407742be792 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a YAML parsing issue in the sandbox-verification GitHub Actions workflow so the workflow file is valid and jobs can be created/executed as expected.
Changes:
- Rewrites the multi-line
python3 -cinvocations used to build JSON request bodies into single-line commands. - Removes zero-indentation lines inside a
run: |block that were causing the workflow YAML to fail parsing (resulting in “0 jobs”).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot this build is failing. The job failed because the backend service did not report healthy within the expected timeout, so all subsequent steps (including obtaining a JWT token and test execution) could not continue. Here’s how to address this: Investigate Backend Service Startup: Check the backend container logs to see why /actuator/health never returned "UP". The backend may fail health checks if a Postgres DB isn’t ready. Ensure DATABASE_URL or related environment variables in backend’s config match what Docker Compose is providing. |
Python does not consume -- as an option terminator; it passes it
through as sys.argv[1]. This caused the JSON body to contain
{"columnName":"--","generatorType":"full_name"} instead of
{"columnName":"full_name","generatorType":"FULL_NAME"}, resulting
in a 400 Bad Request from the API (curl exit code 22).
Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/bd4340f7-88e2-4b27-b879-d21be6992abf
Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
The backend was actually healthy and running fine — the real failure was in the "Add column generators" step (curl exit code 22 = HTTP 400). Root cause: the Fixed in |
|
@copilot the build is failing. This indicates that the code tried to insert a string (character varying) into the "date_of_birth" column (which expects a date type) in the users table. Solution: Ensure that the value for "date_of_birth" in your masking code or data generation logic is a valid date, not a string, or explicitly cast the value to a date. For example, if you use Python for generating values: Python If you currently have:row["date_of_birth"] = "1968-06-10 03:05:52.8941" # String Use this instead:row["date_of_birth"] = datetime.strptime("1968-06-10", "%Y-%m-%d").date() SQL Locate where the masked data is generated or mapped for the 'users' table. |
The BIRTH_DATE generator was returning faker.date().birthday().toString() which produces a String. When PostgreSQLConnector.writeData() passes this to stmt.setObject(), PostgreSQL rejects it with: ERROR: column "date_of_birth" is of type date but expression is of type character varying Fix: return java.sql.Date(faker.date().birthday().time) so JDBC correctly maps it to a SQL DATE type. Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/0451c428-2e84-42ae-b750-404b5345b2a9 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
... The Fixed in |
|
@copilot review GeneratorService.kt and its unit test to ensure all data times return in the correct format, not just strings. see BIRTH_DATE as an example. |
… for all date generators The DATE generator had the same string-returning bug as BIRTH_DATE: it called .toString() on the faker date, producing a String that PostgreSQL rejected for date columns. - Changed DATE generator to return java.sql.Date (matching BIRTH_DATE) - Updated DATE test to assert java.sql.Date type instead of just non-null - Added BIRTH_DATE test asserting java.sql.Date type Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/2f6ca3b8-126f-44ee-940c-2ee61c615b98 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Reviewed all generators in |
…NT generators RANDOM_INT returned .toString() producing a String, causing PostgreSQL to reject inserts into numeric columns (e.g. "salary" of type numeric). Same issue existed in SEQUENTIAL and MONEY_AMOUNT generators. - RANDOM_INT: return Long instead of String - SEQUENTIAL: return Long instead of String - MONEY_AMOUNT: return BigDecimal instead of String - Add assertInstanceOf tests for all three generators Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/888b36ec-98e1-4f51-bff9-4eee4fc169e9 Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
The All three now return proper types:
Added |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| GeneratorType.SEQUENTIAL -> { | ||
| val start = params?.get("start")?.toLongOrNull() ?: 1L | ||
| val step = params?.get("step")?.toLongOrNull() ?: 1L | ||
| val key = columnKey ?: "default" | ||
| val counter = sequentialCounters.computeIfAbsent(key) { AtomicLong(start - step) } | ||
| counter.addAndGet(step).toString() | ||
| counter.addAndGet(step) | ||
| } | ||
| GeneratorType.RANDOM_INT -> { | ||
| val min = params?.get("min")?.toLongOrNull() ?: 1L | ||
| val max = params?.get("max")?.toLongOrNull() ?: 999999L | ||
| faker.number().numberBetween(min, max).toString() | ||
| faker.number().numberBetween(min, max) | ||
| } |
There was a problem hiding this comment.
GeneratorType.SEQUENTIAL and GeneratorType.RANDOM_INT now return Longs (and MONEY_AMOUNT returns BigDecimal). There are existing unit tests in backend/src/test/kotlin/com/opendatamask/application/service/GeneratorServiceCompositeTest.kt that still cast SEQUENTIAL/RANDOM_INT results to String and assert string behavior; those tests will fail and should be updated to match the new return types (or removed if redundant).
|
@copilot apply changes based on the comments in this thread. |
|
@copilot this build is not completed due to failed tests The job failed due to six test failures in GeneratorServiceCompositeTest, all related to either ClassCastException or assertion errors: Key failures:
Solution and code suggestions:
Update GeneratorServiceCompositeTest, especially lines 140, 152, 162, 172, 186, and 191, to ensure type consistency between the expected and actual values. This should resolve the runtime ClassCastException and assertion failures. |
… String Agent-Logs-Url: https://github.com/MaximumTrainer/OpenDataMask/sessions/39c081aa-0cd2-4537-bcbb-b33d79d5f03f Co-authored-by: MaximumTrainer <1376575+MaximumTrainer@users.noreply.github.com>
Updated |
These test failures were already fixed in |
--argument bug in Python invocationsBIRTH_DATEgenerator to returnjava.sql.Dateinstead of StringDATEgenerator to returnjava.sql.Dateinstead of StringRANDOM_INTgenerator to returnLonginstead of String (salary column failing)SEQUENTIALgenerator to returnLonginstead of StringMONEY_AMOUNTgenerator to returnBigDecimalinstead of StringGeneratorServiceCompositeTestto expectLongfor SEQUENTIAL and RANDOM_INT