[BEAM-2524] Update Dataflow FE URLs with regionalized paths.#3455
[BEAM-2524] Update Dataflow FE URLs with regionalized paths.#3455lostluck wants to merge 6 commits into
Conversation
…ers to regionalized paths.
|
Python changes LGTM . There are lint errors (e.g. MonitoringUtil.java:187: Line is longer than 100 characters (found 114). [LineLength]) |
| } | ||
|
|
||
| public static String getJobMonitoringPageURL(String projectName, String jobId) { | ||
| public static String getJobMonitoringPageURL(String projectName, String regionId, String jobId) { |
There was a problem hiding this comment.
I believe this is a backward incompatible change. Someone from the Java team should verify that this interface is not meant to be used by the end users.
There was a problem hiding this comment.
+1 to an expert verification.
I can certainly say that it's only being used outside the Dataflow runner, and certainly not the worker.
If all else fails, I can overload the method and sub in "us-central1" by default which should be sufficient to avoid breaking end user code.
There was a problem hiding this comment.
Let's do what you propose: "overload the method and sub in "us-central1" by default which should be sufficient to avoid breaking end user code"
(and deprecate the old one)
There was a problem hiding this comment.
Agreed with davor; Simultaneously, we make no backwards-compatibility guarantees about this package.
|
Test failure is a flake I think. |
|
jenkints retest this please |
tgroh
left a comment
tgroh
left a comment
There was a problem hiding this comment.
Java LGTM, pending backwards-compatibility override.
| } | ||
|
|
||
| public static String getJobMonitoringPageURL(String projectName, String jobId) { | ||
| public static String getJobMonitoringPageURL(String projectName, String regionId, String jobId) { |
There was a problem hiding this comment.
Agreed with davor; Simultaneously, we make no backwards-compatibility guarantees about this package.
Use old getJobMonitoringPageURL signature as overload. Fill in "us-central1" as the default when it's used.
|
Added using the legacy path as an overload. PTAL |
| @@ -181,13 +181,18 @@ public List<JobMessage> getJobMessages( | |||
| } | |||
|
|
|||
| public static String getJobMonitoringPageURL(String projectName, String jobId) { | |||
There was a problem hiding this comment.
Could you tag the old one as Deprecated?
|
Added @deprecated annotation and @deprecated javadoc preferring use of the other overload. PTAL. |
|
Thank you. LGTM. I will merge after tests pass. |
|
Thanks! |
|
There is a lint error |
5 participants
Updates both the Java and the Python Dataflow Runners with the regionalized form of the Google Cloud Dataflow FE URL in anticipation of Regional Dataflow.
https://console.cloud.corp.google.com/dataflow/jobsDetail/locations/<regionId>/jobs/<jobId>?project=<projectId>