[FOLIO-543] Clicking on users result in pop saying "Column id does not exist" Created: 05/Apr/17 Updated: 12/Nov/18 Resolved: 21/Apr/17 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Bug | Priority: | P2 |
| Reporter: | Adam Dickmeiss | Assignee: | Wayne Schneider |
| Resolution: | Done | Votes: | 0 |
| Labels: | sprint12 | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | 6 hours, 30 minutes | ||
| Original estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||
| Issue links: |
|
||||||||||||
| Sprint: | |||||||||||||
| Description |
|
See attached screenshot |
| Comments |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
Using folio/folio-demo as of today .. There is no version information about modules so this could be hard to reproduce. |
| Comment by shale99 [ 05/Apr/17 ] |
|
hi, this was a change in the database structure as well as in the code. |
| Comment by shale99 [ 05/Apr/17 ] |
|
basically, an sql script with an alter table needs to be run if the tenant isnt re-created. |
| Comment by shale99 [ 05/Apr/17 ] |
|
i updated the major version to indicate this is a breaking change - but am unaware of the guidelines when the db structure has changed |
| Comment by Mike Taylor [ 05/Apr/17 ] |
|
The problem here is in access the patron-groups, which have an ID field called _id rather than the id used by other types of record. (I think this is a hangover from a MongoDB internal convention, and it ought to be fixed.) It looks like you have some UI code that is trying to use ID instead. But that is bizarre – I we've never made that change, and I just re-read the relevant code from git head. So I don't know where your UI code is coming from. Are you on http://folio-uidemo.aws.indexdata.com? I can#t reproduce your issue on that site. |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
I thought for those vagrant images that the database was created each time.. Wayne Schneider might know .. If not, it illustrates the upgrade case issue.. |
| Comment by Mike Taylor [ 05/Apr/17 ] |
|
shale99 I don't understand how a DB structure change could influence what query the software sends. The ui-users code is all written in terms of _id. |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
Mike Taylor I used the installation instructions from https://github.com/folio-org/folio-ansible 's readme .. Thus, the ui and backend stuff was all running locally in Virtualbox (ui on http://localhost:3000) . Perhaps that's not supported or uses outdated stuff? If that's the case our "introduction" to running this system needs an update. |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
Mike Taylor... Zeev changed db structure and changed his code .. But for some reason only one of those changes were part of that vagrant image! This is not in violation of RAML or a web service being screwed .. It's purely internal to RMB. |
| Comment by shale99 [ 05/Apr/17 ] |
|
yes, this is the case, Mike Taylor i changed all _id's in mod-users to id - in the schemas and in the database as well - the previous situation users - usergroups - if the code - if the latest is used, it uses (id) - but if the db hasnt been updated, the _id still exists and you would get the error adam got |
| Comment by shale99 [ 05/Apr/17 ] |
|
this isnt an RMB issue at all in this case |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
shale99: whatever |
| Comment by Mike Taylor [ 05/Apr/17 ] |
|
Adam Dickmeiss: OK, if you are building your own folio-ansible box, that explains why I am not seeing the same problem on the public demo site. Wayne Schneider can comment on whether the folio-ansible build instructions are up to date – or, Adam Dickmeiss, did you mean a different "introduction to running this system" document? shale99, Are you saying the change of ID field is purely internal to the patron-group server-side module, and that it still presents a WSAPI that is in terms of _id? If so, that would explain why I am not seeing the UI-code error I'm looking for. (Also: we should change that WSAPI, when we have a chance to do it in a coordinated way – it's wrong that an old MongoDB convention pollutes our API.) |
| Comment by shale99 [ 05/Apr/17 ] |
|
Mike Taylor |
| Comment by Mike Taylor [ 05/Apr/17 ] |
|
Right. What I am most pragmatically trying to establish is that all of the issues are server side, and the the UI is not implicated – correct? |
| Comment by shale99 [ 05/Apr/17 ] |
|
that is correct. |
| Comment by shale99 [ 05/Apr/17 ] |
|
the only way you might get this do to a UI query would be if you are on an old version of mod-users (before yesterday) and are you sending a cql with id=XYZ |
| Comment by Mike Taylor [ 05/Apr/17 ] |
|
There is no version of the Users UI app that sends CQL using id for a patron-groups query. |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
And indeed I can't see id= in browser network window my browser.. However, grepping for obsolete .
/* TO USE a non jsonb field as a constraint for the join - if for example it is the id field and the id field is of
* type uuid - use the forceCast to cast to a varchar so that it can be compared to a value in a jsonb field that is textual
* JoinBy jbFrom = new JoinBy(UsersAPI.TABLE_NAME_USER, "users", new Criteria().addField("_id").setJSONB(false)
.setForceCast("varchar"), new String[]{"jsonb"});*/
Have no idea whether that was to stay or not. |
| Comment by shale99 [ 05/Apr/17 ] |
|
that is in the comment - should be changed but harmless |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
Good.. Missed that due to long lines.. When a tenant is enabled I thought the system would upgrade the tables to reflect version of current module.. Nobody but the database "knows" its schema. Okapi has no idea what database layout you use. |
| Comment by shale99 [ 05/Apr/17 ] |
|
so if you include the module_from in the json body when calling the tenant api - RMB modules go into upgrade mode - meaning it will do things like - return tenant doesnt exist so why are you trying to upgrade, etc... , but it will also allow a module to declare a "template_update_tenant.sql" as opposed to "template_create_tenant.sql" (which is called for a new tenant) file - and this file will be triggered (run) - so the alter table (in our case) - would sit there and then all would be great - but you would still need to call the tenant api after the upgrade to mod-users v6 |
| Comment by shale99 [ 05/Apr/17 ] |
|
actually, i will add it now, makes sense |
| Comment by Adam Dickmeiss [ 05/Apr/17 ] |
|
Yep.. shale99. that was the idea. I still wonder why we'd have "old" tenant database stuff for the Ansible stuff. I thought that images like that was initiated every time.. |
| Comment by Wayne Schneider [ 05/Apr/17 ] |
|
When a new Vagrant box is released, it is a new build - so the tenant is initialized as part of the build process, there is no upgrade involved. The README in folio-ansible is up to date (in the Quick Start section, which is what I assume Adam was looking at, it just explains how to do a `vagrant init` and a `vagrant up`, pretty straightforward). The box was built yesterday afternoon (my time) using Docker images and NPM packages built from the latest commits of all modules. I am very puzzled, as well, since I tested everything before pushing it up to Atlas. |
| Comment by Wayne Schneider [ 05/Apr/17 ] |
|
I can't replicate the problem with a new build (but it definitely occurs in the folio-demo v0.9.0 Vagrant box in my testing). I also can't figure out what is causing it. I have revoked folio-demo v0.9.0 and folio-backend-auth v0.8.2, and will make new releases. |
| Comment by Adam Dickmeiss [ 06/Apr/17 ] |
|
Since Okapi 1.2.0 - deploy with Docker will pull docker images from remote repositories..
|
| Comment by shale99 [ 06/Apr/17 ] |
|
do we just want to make sure that the tenant api is always called after a major with the needed body? |
| Comment by Wayne Schneider [ 06/Apr/17 ] |
|
I have made a new release of the backend-auth box. I built a new demo box which no longer has the issue Adam reported, but because of a different issue (
|
| Comment by Adam Dickmeiss [ 07/Apr/17 ] |
|
shale99.. major what? upgrade? There is no such thing in Okapi as upgrading a "module". There is a scenario where a Tenant is transitioning from using one Module to another. Yes.. It will most likely be that a Tenant will be using a newer implementation .. This is the tenant call! There must be - in this case at least one running instance of the module with the new version. The issue we have here is that a module has THE SAME MODULE ID. So even the notion of switching a tenant to use another one does not exist. So tenant upgrade will only work if modules have at the very least different module IDs. Before this happens we'll see errors like these over and over. |
| Comment by shale99 [ 09/Apr/17 ] |
|
ok, the workflow would be ? |
| Comment by Adam Dickmeiss [ 18/Apr/17 ] |
|
0. startup instance of a new module.. 1. The tenant init call is invoked before regular "proxy" calls. At this point, the module will consult the storage and see if it can work with existing data. If there's no existing data, it will create the per-tenant module data - whatever that is - with default or omitted values.. If it sees that the existing schema is incompatible with the current module, it can refuse to work with the data. It can also upgrade schema and data. The module owns the data, not Okapi or anybody else.. So it's up to the module maintainer to perform this upgrade. It could also choose NOT to upgrade but work with an old schema.. That would be easy with a schema less system. Perhaps the upgrade is better served with a clone/copy if the database can nof facilitate a rollback in case something goes wrong.. In any case the init call should not return unless the module is happy with the existing data. The tenant init call is important.. It is called with the module is "installed" for the proxy and "upgraded" with the proxy.. Note that even in the "install" case the module will have to consider existing data. Okapi's Tenant upgrade facility is a atomic operation that will move the PROXY vector for one tenant from one module to another.. There's another variant, namely that the maintainer DELETES the tenant-module association , then later associates again.. Okapi has no memory of past module usage. And SHOULD not have.. The module ultimately is responsible for reading the data that it has written in the first place.. In another version. For this reason a module could contain upgrade information for all previous versions so that it can perform all the delta's on its own. |
| Comment by Wayne Schneider [ 21/Apr/17 ] |
|
The comments on this issue have gone a bit afield from the original concern, which actually had to do with how that particular demo box was created, and have been (mostly) resolved as versioning is being formalized for backend modules. I'm closing the issue, but anyone interested in the comments around upgrading modules should probably bookmark it to refer to later. |