Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: endpoint to rename an attribute name in a table #178

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Namyalg
Copy link
Member

@Namyalg Namyalg commented Sep 2, 2022

Pull Request Template

Description

This PR adds an endpoint to rename/modify the attribute name of a table

Fixes #177

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

The functionality of the endpoint has been tested locally

Working

The endpoint is a POST request that takes in the payload as :

 {
      "title" : "name-of-simulation",
       "tables" : { 
              "table_1 : {
                     "old_name_1" : "new_name_1",
                     "old_name_2" : "new_name_2"
               },
               
              "table_2 : {
                     "old_name_1" : "new_name_1",
                     "old_name_2" : "new_name_2"
               }

}

Using cURL, it can be performed as :

  1. Consider the table turnover_parameter, title of simulation as run4
  2. Tableturnover_parameter has the attributes (can be obtained using the endpoint /upload/db/tables ) : ["id","foliage","stems","branch","stem_snag","branch_snag","coarse_root","fine_root","branch_snag_split","coarse_ag_split","fine_ag_split"]

To rename the attribute id to ids, the cURL command is

curl --header "Content-Type: application/json"   --request POST   --data '{"title":"run4","tables" :{"turnover_parameter": {"id" : "ids"}}}'   http://localhost:8080/gcbm/attribute/rename

Erroneous cases handled

  1. Case 1 : When a table passed in the payload does not exist
    Assume the name of the table passed is turn_parameter instead of turnover_parameter, the request will result in
 curl --header "Content-Type: application/json"   --request POST   --data '{"title":"run4","tables" :{"vol_to_bio_factor" : {"a" : "ad"} , "turn_parameter": {"stem" : "stems"}}}'   http://localhost:8080/gcbm/attribute/rename

image

  1. Case 2 : When an attribute name does not exist in a given table

Attribute stems does not exist in turnover_parameter

curl --header "Content-Type: application/json"   --request POST   --data '{"title":"run4","tables" :{"turnover_parameter": {"stems" : "stem"}}}'   http://localhost:8080/gcbm/attribute/rename
{"message":"Error in table turnover_parameter no such column: \"stems\"","status":0}

image

Questions

In the current design, encountering an error of the 2 types will immediately return from the endpoint with the {status : 0, message : error-that-occured} .
This implies, that the changes or queries that were present earlier will be executed.

To provide a robust mechanism, a few other ideas are :

  1. Scan through all of the requests, only if there are no possible errors, execute all of the renaming operations
  2. Scan through each table, and if a particular table has an error, do not perform any rename operation on that table, while execute the operations on other tables
  3. Scan through each table, and if a particular table has an error return an error message immediately, while performing the operations that occur before

What would the best option amongst these ?

@HarshCasper @aornugent @SanjaySinghRajpoot @YashKandalkar I would like to know your opinions

@YashKandalkar
Copy link

Another option is to have SQL transactions for renaming attributes. The python sqlite3 library doesn't seem to support transactions by default, but I found this: https://stackoverflow.com/a/23634805/10047268
We can have transactions by this but I think we'll need to manually execute "commit" after each statement in other endpoints if we make the isolation_level to None

@Namyalg
Copy link
Member Author

Namyalg commented Sep 3, 2022

Thank you @YashKandalkar, will take a look

@HarshCasper
Copy link
Member

@Namyalg — Any updates on this? It would be good to resolve the merge conflicts as well.

@Namyalg
Copy link
Member Author

Namyalg commented Oct 10, 2022

This is not yet resolved

@aornugent
Copy link
Contributor

@Namyalg - do you have time to finish this PR? If not, let's file an issue and try to attract other contributions to finish the featureset.

@Namyalg
Copy link
Member Author

Namyalg commented Dec 24, 2022

I will work on resolving the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Endpoint to modify attribute names in the table
4 participants