[b/402028795] Oozie connector #793

vladislav-sidorovich · 2025-03-17T16:03:45Z

Run Oozie connector:

kinit
export OOZIE_URL=https://....cloudera.site:11443/oozie
./dwh-migration-dumper --connector oozie

The code will be cleaned, extra parameters like url and dates added.

The main flow and tests a ready and can be reviewed.

kaxuna · 2025-03-17T16:07:35Z

dumper/app/build.gradle

@@ -72,6 +72,13 @@ dependencies {
    implementation libs.httpclient5
    implementation libs.aws.java.sdk.redshift
    implementation libs.aws.java.sdk.cloudwatch
+    implementation('org.apache.oozie:oozie-client:5.2.1') {


do we have this and its transient dependencies on airlock?

I have no idea.

if you give me the list of the transients I can check

kaxuna · 2025-03-17T16:09:15Z

dumper/app/build.gradle

-    into layout.buildDirectory.dir('mirror/sources')
-    outputs.dir "mirror/sources"
-}
+//tasks.register('generateSourceMirror', Copy) {


maybe comment why we are not deleting this code?

kaxuna · 2025-03-17T16:11:40Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+
+  protected static String getOozieUrl() {
+    String url = null; // commandLine.getOptionValue(OOZIE_OPTION);
+    if (url == null) {


is not the if statement always true?

kaxuna · 2025-03-17T16:12:26Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+    // String username = commandLine.getOptionValue(USERNAME);
+    // String password = commandLine.getOptionValue(PASSWORD);
+    // if (username != null && password != null) {
+    //   String encoded = Base64.getEncoder().encodeToString((username + ':' + password).getBytes(
+    //       StandardCharsets.UTF_8));
+    //   wc.setHeader("Authorization", "Basic " + encoded);
+    // }


kaxuna · 2025-03-17T16:24:23Z

.../com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTask.java

+      LOG.info("Start fetch Oozie jobs for last {}d starts from {}", maxDaysToFetch, now);
+
+      while (ChronoUnit.DAYS.between(now, lastJobEndDate) < maxDaysToFetch) {
+        List<WorkflowJob> jobsInfo = oozieClient.getJobsInfo(null, offset, batchSize);


does getJobsInfo() returns elements sorted by the endTime date?

sayuzbas-google · 2025-03-17T16:24:19Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+      url = System.getenv(ENV_OOZIE_URL);
+      if (url == null) {
+        throw new IllegalArgumentException(
+            "Oozie URL is not available neither in command option or in the environment");


nor in the

sayuzbas-google · 2025-03-17T16:24:51Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+
+  protected static String getOozieUrl() {
+    String url = null; // commandLine.getOptionValue(OOZIE_OPTION);
+    if (url == null) {


this is always true

sayuzbas-google · 2025-03-17T16:26:42Z

...com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieConnector.java

+@Description("Dumps Jobs history from Oozie.")
+@RespectsInput(order = 100, arg = ConnectorArguments.OPT_URI, description = "Oozie URL.")
+public class OozieConnector extends AbstractConnector implements MetadataConnector {
+  public static final String FORMAT_NAME = "oozie.dump.zip";


can it be private? it is not used anywhere else

sayuzbas-google · 2025-03-17T16:28:05Z

...com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieConnector.java

+      throws Exception {
+    out.add(new DumpMetadataTask(arguments, FORMAT_NAME));
+    out.add(new FormatTask(FORMAT_NAME));
+    out.add(new OozieJobsTask(93));


what is 93?

default quarter days

I would suggest to add a variable or comment for visibility

sayuzbas-google · 2025-03-17T16:32:54Z

.../com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTask.java

+          break;
+        }
+
+        WorkflowJob lastJob = jobsInfo.get(jobsInfo.size() - 1);


Are the jobs inside jobsInfo sorted? So you can take the last element?

sayuzbas-google · 2025-03-17T16:33:59Z

.../com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTask.java

+  public OozieJobsTask(int maxDaysToFetch) {
+    super("jobs.csv");
+    Preconditions.checkArgument(
+        maxDaysToFetch >= 1,


Is it worth also have upper limit for maxDaysToFetch? So it will not get any large numbers, which do not make sense, like million?

kajgol

Any reason why this is under hadoop? The other hadoop connectors (ranger, hdfs) are top-level.

kajgol · 2025-03-18T09:55:51Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+    String url = null; // commandLine.getOptionValue(OOZIE_OPTION);
+    if (url == null) {
+      url = System.getenv(ENV_OOZIE_URL);
+      if (url == null) {


could be moved out from the if to reduce nesting

kajgol · 2025-03-18T09:57:59Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+      String key = (String) entry.getKey();
+      if (key.startsWith(WS_HEADER_PREFIX)) {
+        String header = key.substring(WS_HEADER_PREFIX.length());
+        System.out.println("Header added to Oozie client: " + header);


wouldn't a logger be better?

kajgol · 2025-03-18T09:59:07Z

...google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieClientFactory.java

+
+  protected static void setRetryCount(OozieClient wc) {
+    String retryCount = System.getProperty(OOZIE_RETRY_COUNT);
+    if (retryCount != null && !retryCount.isEmpty()) {


Strings.isNullOrEmpty?

kajgol · 2025-03-18T10:13:40Z

.../com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTask.java

+      LOG.info("Start fetch Oozie jobs for last {}d starts from {}", maxDaysToFetch, now);
+
+      while (ChronoUnit.DAYS.between(now, lastJobEndDate) < maxDaysToFetch) {
+        List<WorkflowJob> jobsInfo = oozieClient.getJobsInfo(null, offset, batchSize);


maybe jobs, jobList or jobBatch would be better names?

kajgol · 2025-03-18T10:14:30Z

.../com/google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTask.java

+    return null;
+  }
+
+  private Object[] toCSVRecord(WorkflowJob job, ImmutableList<String> header) throws Exception {


could be static?

kajgol · 2025-03-18T10:27:19Z

.../google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTaskTest.java

+
+    assertEquals("Amount of days must be a positive number. Got -3.", exception.getMessage());
+
+    new OozieJobsTask(1);


what's this for?

success. No, Errors.

kajgol · 2025-03-18T10:28:10Z

.../google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTaskTest.java

+  }
+
+  @Test
+  public void create_negativeDays_throwsException() throws Exception {


nonpositive instead of negative since you're testing with 0?

kajgol · 2025-03-18T10:32:27Z

...gle/edwmigration/dumper/application/dumper/connector/hadoop/oozie/TestNoAuthOozieClient.java

+import org.apache.oozie.client.AuthOozieClient;
+import org.apache.oozie.client.OozieClientException;
+
+public class TestNoAuthOozieClient extends AuthOozieClient {


Does it need to be public? In fact could it be a private static nested class in the test class?

vladislav-sidorovich · 2025-03-18T11:29:16Z

Any reason why this is under hadoop? The other hadoop connectors (ranger, hdfs) are top-level.

Other connectors also should be moved under hadoop, but not in the scope of this PR.

zaldis · 2025-03-18T15:50:08Z

.../google/edwmigration/dumper/application/dumper/connector/hadoop/oozie/OozieJobsTaskTest.java

+  private static ResponseDefinitionBuilder okJsonWithBodyFile(String fileName) {
+    return ok().withBodyFile(fileName).withHeader(CONTENT_TYPE, "application/json");
+  }


It might be useful to move this method as util method in global scope.

vladislav-sidorovich added 3 commits March 14, 2025 14:12

Oozie connector added

63f2d8e

add missing files

0ac8294

add unit tests for Oozie task and connector

b5717d8

vladislav-sidorovich requested review from sten1ee, zaldis, kaxuna, shevek-google, sayuzbas-google and kajgol March 17, 2025 16:03

kaxuna reviewed Mar 17, 2025

View reviewed changes

sayuzbas-google reviewed Mar 17, 2025

View reviewed changes

kajgol reviewed Mar 18, 2025

View reviewed changes

rename oozie jobs csv file

03de34f

zaldis reviewed Mar 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[b/402028795] Oozie connector #793

[b/402028795] Oozie connector #793

vladislav-sidorovich commented Mar 17, 2025

kaxuna Mar 17, 2025

vladislav-sidorovich Mar 17, 2025

kaxuna Mar 18, 2025

kaxuna Mar 17, 2025

kaxuna Mar 17, 2025

kaxuna Mar 17, 2025

kaxuna Mar 17, 2025 •

edited

Loading

sayuzbas-google Mar 17, 2025

sayuzbas-google Mar 17, 2025

sayuzbas-google Mar 17, 2025

sayuzbas-google Mar 17, 2025

vladislav-sidorovich Mar 18, 2025

zaldis Mar 18, 2025

sayuzbas-google Mar 17, 2025

sayuzbas-google Mar 17, 2025

kajgol left a comment

kajgol Mar 18, 2025

kajgol Mar 18, 2025

kajgol Mar 18, 2025

kajgol Mar 18, 2025

kajgol Mar 18, 2025

kajgol Mar 18, 2025

vladislav-sidorovich Mar 18, 2025

kajgol Mar 18, 2025

kajgol Mar 18, 2025

vladislav-sidorovich commented Mar 18, 2025

zaldis Mar 18, 2025


		assertEquals("Amount of days must be a positive number. Got -3.", exception.getMessage());

		new OozieJobsTask(1);

[b/402028795] Oozie connector #793

Are you sure you want to change the base?

[b/402028795] Oozie connector #793

Conversation

vladislav-sidorovich commented Mar 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaxuna Mar 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kajgol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladislav-sidorovich commented Mar 18, 2025

Choose a reason for hiding this comment

kaxuna Mar 17, 2025 •

edited

Loading