1
\$\begingroup\$

Part 2: Error Handling (and anything else that can be improved)

(Part 1 focused on general cleanup.)


I received this comment in an unrelated post (thanks @Milton):

...Since it is obvious you are using a GIS endpoint, I would note that a lot can go wrong with your script if the GIS were to go down, move url, request gets lost, etc. I discourage url links to GIS data...

I think this concern is legitimate. If this script produces an error, then the user will not be able to save their work orders in the work order management system (Maximo). This would render the entire system unusable until the problem is fixed.

To address this concern, I've put a generic try, except error handler in the main part of the script. Is this good enough, or should it be improved?


The Script:

#What the script does:
#     1. Takes the X&Y coordinates of a work order in Maximo
#     2. Generates a URL from the coordinates
#     3. Executes the URL via a separate script/library (LIB_HTTPCLIENT)
#     4. Performs a spatial query in an ESRI REST feature service (a separate GIS system)
#     5. JSON text is returned to Maximo with the attributes of the zone that the work 
#        order intersected
#     6. The zone number is parsed from the JSON text via a separate script/library 
#        (LIB_PARSE_JSON)
#     7. Inserts the zone number into the work order record
#
#Notes about libraries:
#     - Unfortunately, I'm unable to add external Python libraries (like urllib or 
#       urlparse) to my Maximo/Jython implementation.
#     - Furthermore, some libraries (exampple: JSON) that are normally included in Python
#       and Jython have been excluded from my Maximo implementation. I don't have control
#       over this.
#     - Instead, if there is functionality missing from my Jython implementation, I need 
#       to use Java classes to fill in the gap: 
#       https://www.ibm.com/support/knowledgecenter/ja/SSEUEX_2.0.2/com.ibm.javaeuc.doc/com/ibm/json/java/package-summary.html
#
#Notes about creating the URL:
#     - Previous code reviewers have strongly suggested "...splitting the URL creation 
#       into different variables and adding a looping function that adds query parameters 
#       to the URL."
#     - I've tried doing this, but I found that it made the url parts harder for me to 
#       manage, not easier. Maybe I'm nuts, but I've tried it, and figured it was overly 
#       complicated, so I think we can skip this idea for now. Thanks all the same.

from psdi.mbo import MboConstants
from java.util import HashMap

field_to_update = "ZONE"

def get_coords():
    """
    Get the y and x coordinates(UTM projection) from the WOSERVICEADDRESS table
    via the SERVICEADDRESS system relationship.
    The datatype of the LatitdeY and LongitudeX fields is decimal.
    """
    laty  = mbo.getDouble("SERVICEADDRESS.LatitudeY")
    longx = mbo.getDouble("SERVICEADDRESS.LongitudeX")

    return laty, longx


def is_latlong_valid(laty, longx):
    #Verify if the numbers are legitimate UTM coordinates
    return (4000000 <= laty <= 5000000 and
            600000 <= longx <= 700000)


def make_url(laty, longx):
    """
    Assemble the URL (including the longx and the laty).
    Note: The coordinates are flipped in the url.
    Consider replacing the field wildcard(*) with the specific field name (zone)
    """
    url="http://example.com/arcgis/rest/services/Something/Zones/MapServer/15/query?geometry={0}%2C{1}&geometryType=esriGeometryPoint&spatialRel=esriSpatialRelIntersects&outFields=*&returnGeometry=false&f=pjson".format(str(longx),str(laty))

    return url


def fetch_zone(url):
    # Get the JSON text from the feature service (the JSON text contains the zone value).
    ctx = HashMap()
    ctx.put("url", url)
    service.invokeScript("LIB_HTTPCLIENT", ctx)
    json_text = str(ctx.get("response"))

    # Parse the zone value from the JSON text
    ctx = HashMap()
    ctx.put("json_text", json_text)
    service.invokeScript("LIB_PARSE_JSON", ctx)
    parsed_val = str(ctx.get("parsed_val"))

    return parsed_val


#Is this sort of generic error checking acceptable (try/except)?
try:
    laty, longx = get_coords()
    if not is_latlong_valid(laty, longx):
        service.log('Invalid coordinates')
    else:
        url = make_url(laty, longx)
        zone = fetch_zone(url)

        #Insert the zone value into the zone field in the work order
        mbo.setValue(field_to_update, zone, MboConstants.NOACCESSCHECK)
except:
    """
    If the script fails, then set the field vaule to null.
    Reason: If the work order coordinates have changed (thereby triggering this script), 
    then setting the zone's field value to null is better than leaving it as the wrong 
    zone number (if the coordinates of the work order changed, that means
    that the zone number likely changed too).
    Furthermore, if there is an error in the script, then an error message will pop up 
    and prevent users from creating/saving work orders. 
    We don't want that! (error messages would render the system unusable until fixed)
    So, I figure, I'll set the field value to null if there is an error.
    """
    mbo.setValue(field_to_update, "", MboConstants.NOACCESSCHECK) #Should I
    # set the field value to "" or to None? 
    service.log("An exception occurred")

LIB_HTTPCLIENT:

from psdi.iface.router import HTTPHandler
from java.util import HashMap
from java.lang import String

handler = HTTPHandler()
map = HashMap()
map.put("URL", url)
map.put("HTTPMETHOD", "GET")
responseBytes = handler.invoke(map, None)
response = String(responseBytes, "utf-8")

LIB_PARSE_JSON:

#Is there a way to NOT hardcode the field name (ZONE)?

from com.ibm.json.java import JSONObject

obj = JSONObject.parse(json_text)
#The "features" element of the JSON object has an array. Get the first feature in the
#array by specifying [0].
parsed_val = obj.get("features")[0].get("attributes").get("ZONE")
\$\endgroup\$
  • 1
    \$\begingroup\$ If you use the Contact link at the bottom of the page, staff could delete/edit the bad versions of the code from the edit history. Please remember to ask them to fix both question and answer if you do that. It's the same process as would be used for login credentials, although you might want to select Other instead. \$\endgroup\$ – mdfst13 Aug 26 '19 at 12:04
1
\$\begingroup\$

The code has certainly improved since I last saw it. There's only one thing that stands out to me, and it's the URL construction. The 'simple' way to make it more legible is to split it up onto multiple lines using implicit concatenation:

url = (
    "http://example.com"
    "/arcgis/rest/services/Something"
    "/Zones/MapServer/15/query?"
    "geometry={0}%2C{1}&"
    "geometryType=esriGeometryPoint&"
    "spatialRel=esriSpatialRelIntersects&"
    "outFields=*&"
    "returnGeometry=false&"
    "f=pjson"
).format(longx, laty)

Note that the str calls have also been removed; the format call does that for you.

Also, and this is domain-specific so I can't give any specific advice, but: if you're able to narrow outFields so that you get only the fields back that you need from the server, that will be more efficient than *.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you! Will implement your suggestions. Any thoughts on NOT hard coding the field name (ZONE) in the LIB_PARSE_JSON script? \$\endgroup\$ – User1973 Aug 26 '19 at 2:28
  • 1
    \$\begingroup\$ Yes. The invoke script mechanism is kind of goofy. Your two libraries are so small that they deserve to be regular Python functions inside of your main script, if that's possible. Then you can accept the field name as a parameter. \$\endgroup\$ – Reinderien Aug 26 '19 at 3:38
  • 1
    \$\begingroup\$ The DRY principle suggests that keeping the libraries as libraries is a good idea, as otherwise the code will have to be repeated in any other scripts that need to make similar calls. Really, in Maximo terms, library scripts should be thought of as global functions -- admittedly with a clunky invocation syntax. \$\endgroup\$ – Preacher Aug 26 '19 at 20:31
  • 1
    \$\begingroup\$ For what it's worth: the latest version on the code is here: stackoverflow.com/a/57730617/10936066 \$\endgroup\$ – User1973 Sep 4 '19 at 12:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.