[STCLI-148] optional-dependency syntax in MDs is incorrect Created: 23/Apr/20 Updated: 24/Apr/20 Resolved: 23/Apr/20 |
|
| Status: | Closed |
| Project: | stripes-cli |
| Components: | None |
| Affects versions: | None |
| Fix versions: | 1.15.1 |
| Type: | Bug | Priority: | P2 |
| Reporter: | Zak Burke | Assignee: | Mike Taylor |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||
| Sprint: | |||||||||||||||||||||
| Description |
|
CI cannot complete`master` branch builds of ui-tenant-settings because module descriptor publishing fails. It appears not to like the new `optionalOkapiInterfaces` that are now present in the `package.json`:
"optionalOkapiInterfaces": {
"course-reserves-storage": "0.2",
"reserves-storage": "0.1"
},
as the build log reports: HttpMethod: POST URL: http://folio-registry.aws.indexdata.com/_/proxy/modules Content-Type: application/json; charset=UTF-8 Accept: application/json Sending request to url: http://folio-registry.aws.indexdata.com/_/proxy/modules Response Code: HTTP/1.1 400 Bad Request Response: Failed to decode:Unrecognized field "optional" (class org.folio.okapi.bean.InterfaceDescriptor), not marked as ignorable (5 known properties: "version", "handlers", "id", "scope", "interfaceType"]) |
| Comments |
| Comment by Zak Burke [ 23/Apr/20 ] |
|
Mike Taylor, any thoughts? I know you did some related work in stripes-cli for
|
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
Agreed, this seems to be a problem in Okapi. |
| Comment by Marc Johnson [ 23/Apr/20 ] |
|
The error reported in the issue suggests that Okapi is refusing the descriptor because it does not recognise a property. That either means that:
John Malconian Ian Hardy What version of Okapi is the registry running? Is it 2.37.0 or later? |
| Comment by Marc Johnson [ 23/Apr/20 ] |
|
Please can an example module descriptor be included. The error message suggests that the optional property is being included at the same level as version, handlers, id, scope, interfaceType which are properties of the interface descriptor schema (used for both provided interfaces and dependencies). Is it being included as a boolean? The Module Descriptor schema suggests that this is actually modelled by a separate array object named optional at a higher level. This automated test seems to confirm that. That suggests to me that the client, the UI module build, is generating an invalid module descriptor and Okapi is correct in refusing it. |
| Comment by Zak Burke [ 23/Apr/20 ] |
|
Marc Johnson, the MD from linked build failure is copied below. Mike Taylor, does Marc's assessment make sense?
{
"id": "folio_tenant-settings-3.0.100096",
"name": "Tenant settings",
"permissionSets": [
{
"permissionName": "module.tenant-settings.enabled",
"displayName": "UI: Tenant-settings module is enabled"
},
{
"permissionName": "settings.tenant-settings.enabled",
"displayName": "Settings (tenant): display list of settings pages",
"subPermissions": [
"settings.enabled"
]
},
{
"permissionName": "ui-tenant-settings.settings.addresses",
"displayName": "Settings (tenant): Can manage tenant addresses",
"subPermissions": [
"configuration.all",
"users.collection.get",
"settings.tenant-settings.enabled"
],
"visible": true
},
{
"permissionName": "ui-tenant-settings.settings.key-bindings",
"displayName": "Settings (tenant): Can maintain key bindings",
"subPermissions": [
"configuration.all",
"settings.tenant-settings.enabled"
],
"visible": true
},
{
"permissionName": "ui-tenant-settings.settings.locale",
"displayName": "Settings (tenant): Can edit language, localization, and currency",
"subPermissions": [
"configuration.all",
"settings.tenant-settings.enabled"
],
"visible": true
},
{
"permissionName": "ui-tenant-settings.settings.plugins",
"displayName": "Settings (tenant): Can maintain preferred plugins",
"subPermissions": [
"configuration.all",
"settings.tenant-settings.enabled"
],
"visible": true
},
{
"permissionName": "ui-tenant-settings.settings.sso",
"displayName": "Settings (tenant): Can maintain SSO settings",
"subPermissions": [
"configuration.all",
"settings.tenant-settings.enabled"
],
"visible": true
},
{
"permissionName": "ui-tenant-settings.settings.location",
"displayName": "Settings (tenant): Can create, edit and remove locations",
"subPermissions": [
"configuration.all",
"settings.tenant-settings.enabled",
"users.item.get",
"users.collection.get",
"inventory-storage.location-units.institutions.collection.get",
"inventory-storage.location-units.institutions.item.get",
"inventory-storage.location-units.institutions.item.post",
"inventory-storage.location-units.institutions.item.put",
"inventory-storage.location-units.institutions.item.delete",
"inventory-storage.location-units.campuses.collection.get",
"inventory-storage.location-units.campuses.item.get",
"inventory-storage.location-units.campuses.item.post",
"inventory-storage.location-units.campuses.item.put",
"inventory-storage.location-units.campuses.item.delete",
"inventory-storage.location-units.libraries.collection.get",
"inventory-storage.location-units.libraries.item.get",
"inventory-storage.location-units.libraries.item.post",
"inventory-storage.location-units.libraries.item.put",
"inventory-storage.location-units.libraries.item.delete",
"inventory-storage.locations.collection.get",
"inventory-storage.locations.item.get",
"inventory-storage.locations.item.post",
"inventory-storage.locations.item.put",
"inventory-storage.locations.item.delete"
],
"visible": true
},
{
"permissionName": "ui-tenant-settings.settings.servicepoints",
"displayName": "Settings (tenant): Can create, edit and remove service points",
"subPermissions": [
"configuration.all",
"settings.tenant-settings.enabled",
"users.item.get",
"users.collection.get",
"inventory-storage.locations.collection.get",
"inventory-storage.location-units.institutions.collection.get",
"inventory-storage.location-units.campuses.collection.get",
"inventory-storage.location-units.libraries.collection.get",
"inventory-storage.service-points.collection.get",
"inventory-storage.service-points.item.get",
"inventory-storage.service-points.item.post",
"inventory-storage.service-points.item.put",
"inventory-storage.service-points.item.delete"
],
"visible": true
}
],
"requires": [
{
"id": "configuration",
"version": "2.0"
},
{
"id": "location-units",
"version": "2.0"
},
{
"id": "locations",
"version": "3.0"
},
{
"id": "login-saml",
"version": "1.0"
},
{
"id": "service-points",
"version": "3.0"
},
{
"id": "users",
"version": "15.0"
},
{
"id": "course-reserves-storage",
"version": "0.2",
"optional": true
},
{
"id": "reserves-storage",
"version": "0.1",
"optional": true
}
]
}
|
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
Note that you can easily see generated descriptors for yourself: |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
But, yes, Marc's analysis looks right. It seems that what the Stripes CLI generates is not the format that Okapi expects. I can fix that. |
| Comment by Marc Johnson [ 23/Apr/20 ] |
|
Should this issue be moved to the project for stripes-cli? |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
Too late, I fixed it: https://github.com/folio-org/stripes-cli/pull/218 |
| Comment by John Malconian [ 23/Apr/20 ] |
|
the FOLIO registry is running okapi 2.38.0. |
| Comment by Marc Johnson [ 23/Apr/20 ] |
|
I think this can be closed as cannot reproduce, the issue was with the stripes-cli not Okapi, and Mike has already worked on a change for that separately |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
Don't you think that verifying that my fix works is part of the present issue? |
| Comment by Marc Johnson [ 23/Apr/20 ] |
If you want to use this issue to manage your fix, then I don't mind you re-opening it. If you do, please could it be moved to the appropriate project and updated the reflect that the cause. |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
Well, I'm not going to get involved in devops work, which is what this is now. I guess if you guys want to close this and open another, I won't stand in your way. |
| Comment by Marc Johnson [ 23/Apr/20 ] |
I don't follow. How is this DevOps work? |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
First line of the description: "CI cannot complete`master` branch builds of ui-tenant-settings". That's devops, isn't it? The obvious way to check that the new format is correct would be to re-do the CI `master` branch build of ui-tenant-settings. |
| Comment by Marc Johnson [ 23/Apr/20 ] |
My understanding is that was a description of the symptoms of the issue. Zak Burke may have had an inkling that the cause was the CI process or Okapi (hence the issue being posted to the #devops and #development channels for help). Given that we now know that the cause is that ui-tenant-settings (using stripes-cli) was generating an invalid descriptor (and hence Okapi and CI were doing the right thing), I would say that either this issue be closed as not relevant to the CI process or Okapi, or that it be moved to the project where the issue and the fix can be done.
Personally, I think the best way to check that this is resolved would be to validate that the generated descriptor is valid. I think that can be done without any involvement of the CI process. I think it's best I stop, I likely shouldn't have asked the previous question. I don't think I'm adding any value to you or Zak Burke's day by prolonging this. Apologies |
| Comment by Mike Taylor [ 23/Apr/20 ] |
Yes; but not without me building and running an Okapi (or someone else doing it). Anyway, let's wait for Zak Burke to turn up here and propose a way forward. |
| Comment by Marc Johnson [ 23/Apr/20 ] |
|
Apologies to all, I should've left this issue open and stepped away once I'd helped diagnose the issue |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
I reject your apology! |
| Comment by Zak Burke [ 23/Apr/20 ] |
|
The original description is the behavior I saw in Jenkins (hence filing this as a FOLIO ticket and notifying #dev-ops). But then I realized the problem was not a process problem but that okapi was rejecting the generated MD (hence asking if this should be moved to OKAPI and notifying #development). But then we realized this was a problem with stripes-cli incorrectly generating MDs, which were generated during Jenkins builds, and then passed to Okapi, hence this step: I'm going to edit the description, re-open it, and move it to stripes-cli. Whether we should have opened new tickets at each stage of the diagnosis or moved this one along is a question we deal with all the time, e.g. "some UI app shows some glitch which is a result of data it received from the backend and formatted with a component from stripes-components, so which should the ticket be filed against?" There is more than one possible answer, no single right answer, and since the right people were notified and we got it fixed, I'm not gonna worry about it further |
| Comment by Marc Johnson [ 23/Apr/20 ] |
Agreed, I removed a chunk of one of my replies because I didn't want to spend time on that topic again (we've been around that merry go round too many times recently) |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
So just to clarify where we landed: Zak Burke you reassigned this to me. Does that mean it's on me to make the release of stripes-cli? |
| Comment by Zak Burke [ 23/Apr/20 ] |
|
Nope; Ryan Berger is making the release. Sorry for the confusion. I assigned it to you since you wrote the PR that fixes the actual bug in stripes-cli. I figured we'd close it once we verify it in the ui-tenant-settings build pipeline. I could have written another Jira to cover the process of making the release so we could test it against ui-tenant-settings but that felt like overkill. |
| Comment by Mike Taylor [ 23/Apr/20 ] |
|
Strong agreement that another issue would be overkill I'm off to review Ryan Berger's PR. |
| Comment by Zak Burke [ 23/Apr/20 ] |
|
Tested against ui-tenant-settings and its output is now correct for optionalOkapiInterfaces. |
| Comment by Mike Taylor [ 24/Apr/20 ] |
|
Great news! Sorry for my initial screwup that made this necessary. |