hashmapinc / Drillflow

A dockerized WITSML API Server that is agnostic of the backend.
Apache License 2.0
17 stars 13 forks source link

DEV: Parent Card- Determine Code Impact to client PUT API Change #619

Closed niaalex closed 5 years ago

niaalex commented 5 years ago

DEV: Analysis: small (search codebase for usage & determine code change) DEV: Code & Test: TBD

Per client, the current PUT API of some DoT services provides partial update behavior, which is wrong (The correct PUT behavior is to create a new resource or replace whole existing resource). This is DoT technical debt and we’re going to correct them. Below PUT APIs behavior needs correction:

PUT /witsml/wells/{uid} PUT /witsml/wellbores/{uid} PUT /witsml/trajectories/{uid}

Before any code change on DoT side, could you please confirm whether Drillflow relies on above APIs partial update? If yes, please adjust code accordingly. After that, we will update DoT code.

Thanks, Xiaobin

If impacted Subsequent cards will be created

TessForGithub2 commented 5 years ago

UPDATE: First, I have to find where in the codebase these APIs are being used. Once I determine where & how they are used, I can determine the change(s) (if any) required to block partial updates. This may mean removing the use of the endpoints (which means which endpoints need to be used instead, if any) OR modified use of the endpoints (for example, only allow use of the endpoint if it is a full update (and what does that mean, exactly).

So the first step is to investigate the use of the endpoints within the codebase to understand usage. This is a small effort that just needs to be fit into our workload. Then, depending upon that analysis, further timing can be assigned.

niaalex commented 5 years ago

No ETA at this time. @TessForGithub2 will update by 7/5.

TessForGithub2 commented 5 years ago

UPDATE: PUT is being used for all non-LOG objects (so Well, Wellbore, and Trajectory).

With LOG (refer to my Log Update design document which outlines these REST calls), I do: channelSetRequest = Unirest.patch(channelSetEndpoint); channelsRequest = Unirest.post(channelsEndpoint); channelDataRequest = Unirest.post(dataEndpoint); while all other objects do: HttpRequestWithBody request = Unirest.put(endpoint);

So the answer is, "YES, we are impacted in our code base". Next, I will look at the effort required to change the PUT to a PATCH.

TessForGithub2 commented 5 years ago

UPDATE:

WELL -- Currently use endpoint ...well/v2/witsml/wells/ as PUT Refer to the SLB demo website: democore/Slb.Prism.Core.Service.Well-2 which states: "Create a well object with uid specified or Update a well object in 1.4.1.1 format" (NOTE: this was designed to accommodate both a creation or an update.) Need to use endpoint ...well/v2/witsml/wells/ as PATCH Refer to the SLB demo website: democore/Slb.Prism.Core.Service.Well-2 which states: "Patch well in 1.4.1.1 format"

ESTIMATE OF CODE IMPACT: Based on the following facts, I estimate this as MEDIUM:

  1. The same endpoint is used, just a PATCH vs. PUT.
  2. However, the SLB schema for PATCH is significantly less defined & there appears to be fewer elements in the PATCH (therefore, some type of data mapping and/or data validation may need to be performed to successfully use this endpoint).
niaalex commented 5 years ago

The analysis is done and code impact is determined. @Mike-d-s will need to advise if the team should make a code change.

TessForGithub2 commented 5 years ago

UPDATE: Tech debt for wells & well bores. Still need to have Mike advise the priority.

niaalex commented 5 years ago

@Mike-d-s please confirm how this should be classified

Mike-d-s commented 5 years ago

Closing the parent card as the child cards are in the object backlogs.