BHoM_Adapter - Issues 43/45/63 - Refresh Interface for the adapter : config, params and error log

[adecler]

Fixes #43.
Fixes #45.
Fixes #63.

Depends on BHoM/BHoM#361 and (not really) on BHoM/BHoM_Engine#736.

I would also have liked to take care of #32 but it seems that this one might need more than cosmetic changes so I left it be for now.

My strategy for this change is to use a single CrudConfig object as interface regardless of the type of crud operation being done. The reason for this is to avoid the headache of going from Push/Pull/etc configs to Create/Read/etc configs in a way that is not completely confusing for everyone. This is not an interface because it already has the Replace vs UpdateOnly property expected by the generic adapter.

The GitHub_Toolkit i a nice one to test the benefits of this new approach since it is the only one that I know that really makes use of the config. Unfortunately, there is already a pull request on the repo so I'll jut show you the code here in comments below.

Other than the need to modify the interface of every single adapter, the changes needed are really small. here's the list of manipulation we have on the config currently (only repos for OS listed):

• RevitAdapter:

  • Just create config if null but not used

• GSA_Toolkit:

  • Execute is using parameters dictionary

• Robot_Toolkit:

  • Execute is using parameters dictionary

• Mongo_Toolkit:

  • Execute is using parameters and config dictionary
  • Push is using config for Tag and Replace

• BHoM_UI:

  • Need to change the interface for all CRUD components

• Dynamo_Toolkit:

  • Execute old method is doing some preporcessing (Method_Basilisk/CRUD/Execute)
  • Same for UpdateProperty, Delete, Move, Pull, Push

• Grasshoper_Toolkit

  • SolveInstance of old components are collectiong dictionaries for all CRUD config

Note that we should not change the interface of the old UI components to avoid losing compatibility. Those components can just be modified sightly internally to produce a CrudConfig from the dictionary given how little that config was used anyway.

I have marked this PR for the 2.1 milestone since I think the change should happen soon but it shouldn't necessarily be for the OS deadline. What do you think?

[al-fisher]

This looks really good as a proof of concept. Thanks
As discussed with @adecler blocking merging until new year and further coordination across adapter_tookits

[rwemay]

@adecler , speaking to @IsakNaslundBh , we think this branch/PR is now superseded/other developments have overtaken. Can we close this now and delete the branch?

[al-fisher]

Yep - thanks @rwemay
Following good workshops last week - @adecler @alelom - do perhaps do a final review and close and delete branch when happy now

[alelom]

I was just now in the process of reviewing what could be saved of this / if anything overlaps with the current proposals. Especially with regards to the config which we did not cover much in our chats.
I'll take care of removing this as soon as I think it's not useful anymore. Thanks!

[al-fisher]

Awesome! Thanks @alelom

[alelom]

After some consideration, I think I will take a hint from this parked branch with regards to the config parameter, as illustrated here.

[alelom]

Closing as permanently parked. Keeping the branch as it can be useful as a comparison.