[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: PNG File Screenshot from 2017-04-05 12-35-27.png    
Issue links:
Relates
relates to FOLIO-545 Current auth-blackbox (folio-backend-... Closed
relates to FOLIO-544 Trying out ansible demo Closed
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.
i mistakenly thought that when the code is refreshed in the demo the tenants are re-created.
how are we handling this type of update at this point in the project?

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 -
json schema - id
database - _id

usergroups -
json schema and database - _id

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
the error basically indicates that the "id" is being requested, but that it does not exist and asks if you (the code) means _id (because that is the column that actually exists in the database. this means the new code is running against the old database configuration.
If a new tenant was created , then it would be synced up - id - id --> you will see a change as the _id you were getting back in the json will be changed to id. this was accompanied by a major version bump

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 .
I stil see one _id in mod-users - UserGroupAPI.java:

       /* 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
however, i dont know how we want to handle this at this point in the project - i can easily add this to the "template_update_tenant.sql"

Comment by shale99 [ 05/Apr/17 ]

actually, i will add it now, makes sense
please note, that when upgrading a major version - the create tenant api should be called with the module_from and module_to (json in the body)

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.. OKAPI-283 Closed .. This will, for the Vagrant break thiings because the database previously was using old schema and now after upgrade will use new schema. I was able to reproduce the problem with folio-demo 0.8.1 and upgrade Okapi via apt-get upgrade..

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?
this will start us on the real process of updates

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 ( STRIPES-301 Closed ), the permissions don't display in the Users app, so I haven't released it yet.

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,
can you explain the upgrade workflow then, i may have misunderstood it (lets look beyond the same module id for a minute)

the workflow would be ?
0. startup a new instance for a new version of a module
1. read only mode on the prod db - for specific shemas (of this modules for a specific tenant) - this may be a technical problem
2. migrate / clone (if possible) the schema with the tenant's data to a new db
3. update the tenant's schema with the new db structure (constraints, column renames, whatever...)
a. how does this happen? where does the script that updates an existing schema live - isnt that in the tenant api - with the module_to / module_from indicators - so that the module runs the script to update an existing schema for a tenant to the structure needed by the current module version <- this is how i implemented it, so if not , lets discuss.

Comment by Adam Dickmeiss [ 18/Apr/17 ]

0. startup instance of a new module..
Yep.. A new instance of a module is running. It will listen on the HTTP port, setup connections to a database server to check that fundamental permissions are ok etc.
What does the module know at this point? It knows its own version. It also knows what data it can work with, such as schema

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.

Generated at Thu Feb 08 23:06:34 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.