fix issue #332: updated to return JobRunRow

[Violet-XiaoWeiHuang]

HI @wslulciuc,
please review this Return JobRunRow on JobRunRowMapper.map(), for #332
Tx

[wslulciuc]

@Violet-XiaoWeiHuang, thanks for the help! I noticed the CI build is failing. Are you able to run the following cmd successfully locally on your machine?

$ ./gradlew compileJava

[Violet-XiaoWeiHuang]

No,
`> Task :compileJava FAILED

FAILURE: Build failed with an exception.

• What went wrong:
  Execution failed for task ':compileJava'.

Could not find tools.jar. Please check that /usr/lib/jvm/java-8-openjdk-amd64 contains a valid JDK installation.

• Try:
  Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.

• Get more help at https://help.gradle.org

Deprecated Gradle features were used in this build, making it incompatible with Gradle 5.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/4.10.3/userguide/command_line_interface.html#sec:command_line_warnings

BUILD FAILED in 1s
1 actionable task: 1 executed`

[wslulciuc]

Are you sure you have the JDK installed? see: https://openjdk.java.net/install

[Violet-XiaoWeiHuang]

yeah,
I tried you link to update to install the openjdk.
When I tried ./gradlew compileJava, it shows the same issue. I consider my Ubuntu is built on top of W10, I had to use the server to run the test.

I digged further about JobRun and JobRunRow. They have different members, I cannot just change the return to new JobRunRow
@wslulciuc

[wslulciuc]

I digged further about JobRun and JobRunRow. They have different members, I cannot just change the return to new JobRunRow

Yeah, updating the return type of the mapper is slightly more involved than a simple class rename. The issue doesn't go into the specifics, but you'd eventually run into complication errors once you renamed the class. We want to make sure any class using the mapper are also update along with your PR.

Let me know if you need help on next steps, or if you got it from here. Happy to clarify if needed!

[Violet-XiaoWeiHuang]

I want continue on it.
I compare the members in JobRunRow() and JobRunRowMapper.map(), some of them are different.

marquez/src/main/java/marquez/db/models/JobRunRow.java

Lines 33 to 41 in 99e3285

	@Getter @NonNull private final UUID uuid;
	@Getter @NonNull private final Instant createdAt;
	@Getter @NonNull private final Instant updatedAt;
	@Getter @NonNull private final UUID jobVersionUuid;
	@Getter @NonNull private final String currentRunState;
	@Getter @NonNull private final List<UUID> inputDatasetVersionUuids;
	@Getter @NonNull private final List<UUID> outputDatasetVersionUuids;
	private final Instant nominalStartTime;
	private final Instant nominalEndTime;

marquez/src/main/java/marquez/db/mappers/JobRunRowMapper.java

Lines 31 to 38 in 434b2ba

	results.getObject(Columns.ROW_UUID, UUID.class),
	results.getInt(Columns.CURRENT_RUN_STATE),
	results.getObject(Columns.JOB_VERSION_UUID, UUID.class),
	results.getString(Columns.RUN_ARGS_CHECKSUM),
	results.getString(Columns.RUN_ARGS),
	results.getTimestamp(Columns.NOMINAL_START_TIME),
	results.getTimestamp(Columns.NOMINAL_END_TIME),
	results.getTimestamp(Columns.CREATED_AT));

How could i deal with them? Thank you. @wslulciuc

[wslulciuc]

I compare the members in JobRunRow() and JobRunRowMapper.map(), some of them are different.

@Violet-XiaoWeiHuang: Currently, there are fields in JobRun that are specific to a job run row (job_version_uuid, row_uuid, etc). That's not ideal, and they should be removed from JobRun and only be contained within a JobRunRow. I'm also realizing, as I'm thinking about the sort of changes required to merge this feature, it's fairly involved and requires knowledge of our DB schema. I personally feel this isn't an issue we are ready to tackle just yet, and one that requires far more detail than is outlined in #332 (mostly likely an epic).

In the meantime, I recommend reviewing our DB schema and app structure (see below). Would you be interested in picking up another issue?

Resources

• Marquez Data Model: https://www.lucidchart.com/documents/view/f918ce01-9eb4-4900-b266-49935da271b8/0
• Marquez App Structure: https://docs.google.com/document/d/17qrJNFx5YfFWiSNqd53lMed-YOfjTeZUhkis3rAGbN0

[wslulciuc]

@Violet-XiaoWeiHuang: I'm going to close this PR for now, but we'll want add these changes (eventually). Thank you again for helping out and tacking the issue!