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.

Sign up for GitHub

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

TCR-46 mod-linked-data #83

Merged
merged 7 commits into from
Jan 20, 2025
Merged

TCR-46 mod-linked-data #83

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d0df09f
Evaluate TCR-46 mod-linked-data
maccabeelevine Nov 22, 2024
a8e2381
API documentation is fixed.
maccabeelevine Dec 6, 2024
cad95c6
Interface paths are now prefaced with /linked-data/
maccabeelevine Dec 6, 2024
7a3af2a
Link to sonarqube results
maccabeelevine Dec 16, 2024
f303304
PC approved the module's functionality
maccabeelevine Jan 3, 2025
3a18b11
The three module dependencies were approved
maccabeelevine Jan 6, 2025
1a44b21
Note approval of new libraries
maccabeelevine Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions module_evaluations/TCR-46-2024-11-14-mod-linked-data.MD
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Module acceptance criteria template

## Module Name
mod-linked-data

## How to use this form
When performing a technical evaluation of a module, create a copy of this document and use the conventions below to indicate the status of each criterion. The evaluation results should be placed in the [module_evaluations](https://github.com/folio-org/tech-council/tree/master/module_evaluations) directory and should conform to the following naming convention: `{JIRA Key}_YYYY-MM-DD-{module name}.MD`, e.g. `TCR-1_2021-11-17-mod-foo.MD`. The date here is used to differentiate between initial and potential re-evaluation(s). It should be the date when the evaluation results file was created.

* [x] ACCEPTABLE
* [x] ~INAPPLICABLE~
* [ ] UNACCEPTABLE
* comments on what was evaluated/not evaluated, why a criterion failed

## [Criteria](https://github.com/folio-org/tech-council/blob/7b10294a5c1c10c7e1a7c5b9f99f04bf07630f06/MODULE_ACCEPTANCE_CRITERIA.MD)

## Administrative
* [ ] Listed by the Product Council on [Functionality Evaluated by the PC](https://wiki.folio.org/display/PC/Functionality+Evaluated+by+the+PC) with a positive evaluation result.
- PC will be reviewing this module on December 12th.

## Shared/Common
* [X] Uses Apache 2.0 license
* [X] Module build MUST produce a valid module descriptor
* [X] Module descriptor MUST include interface requirements for all consumed APIs
* [X] Third party dependencies use an Apache 2.0 compatible license
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List of third party dependencies: https://julianladisch.github.io/folio-dependencies/mod-linked-data-1.0.1-SNAPSHOT.html
Can you elaborate how you came to the conclusion that they are compatible?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used one of those npx tools to generate the list of dependencies, and then compared it against the list at https://apache.org/legal/resolved.html, and there were no issues. This was a month ago so I don't remember the specifics of each license used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npx is for JavaScript, not for maven pom.xml.
Can you specify which tool you've used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the catch -- my not noticing that the method we use for UI modules would not apply.

As a different approach, seeing we already use MojoHaus plugins, I added the license-maven-plugin to the pom

      <plugin>
        <groupId>org.codehaus.mojo</groupId>
        <artifactId>license-maven-plugin</artifactId>
        <configuration>
          <outputDirectory>${project.build.directory}/reports</outputDirectory>
        </configuration>
      </plugin>

Then executed the third-party-report goal which generated an HTML page with the results -- 321 total dependencies. The HTML is verbose (no consolidated list of licenses), so I ran this cludgy one-liner on in the page's dev console to generated a sorted, unique list of the licenses.

new Set(Array.from(document.querySelectorAll('#third-parties-list + table > tbody > tr > td:last-child')).map((a) => a.innerText).sort())

The resulting list of 46 "unique" licenses (really a lot fewer, things are spelled differently) is

    "Apache 2",
    "Apache 2.0",
    "Apache License 2.0",
    "Apache License 2.0\nLGPL 2.1\nMPL 1.1",
    "Apache License, 2.0\nBSD 2-Clause\nEDL 1.0\nEPL 2.0\nGPL2 w/ CPE\nMIT license\nModified BSD\nPublic Domain\nW3C license\njQuery license",
    "Apache License, 2.0\nEPL 2.0\nPublic Domain\nThe GNU General Public License (GPL), Version 2, With Classpath Exception",
    "Apache License, 2.0\nEPL 2.0\nThe GNU General Public License (GPL), Version 2, With Classpath Exception",
    "Apache License, Version 2.0",
    "Apache Public License 2.0",
    "Apache Software License, version 2.0\nLesser General Public License, version 3 or greater",
    "Apache-2.0",
    "Apache-2.0\nLGPL-2.1-or-later",
    "BSD",
    "BSD 2-Clause License",
    "BSD License 3",
    "BSD-2-Clause",
    "BSD-2-Clause\nPublic Domain, per Creative Commons CC0",
    "BSD-3-Clause",
    "Bouncy Castle Licence",
    "CDDL\nGPLv2+CE",
    "CDDL 1.1\nGPL2 w/ CPE",
    "EDL 1.0",
    "EPL 2.0\nGPL2 w/ CPE",
    "Eclipse Distribution License - v 1.0",
    "Eclipse Distribution License v. 1.0\nEclipse Public License v. 2.0",
    "Eclipse Public License - v 1.0\nThe Apache Software License, Version 2.0",
    "Eclipse Public License - v 2.0",
    "Eclipse Public License - v 2.0\nThe Apache Software License, Version 2.0",
    "Eclipse Public License 1.0",
    "Eclipse Public License v. 2.0\nGNU General Public License, version 2 with the GNU Classpath Exception",
    "Eclipse Public License v2.0",
    "Eclipse Public License, Version 2.0",
    "GNU Lesser General Public License, Version 2.1",
    "GNU Library General Public License v2.1 or later",
    "LGPL",
    "MIT",
    "MIT License",
    "Mozilla Public License, Version 2.0",
    "Public Domain",
    "Public Domain, per Creative Commons CC0",
    "The (New) BSD License",
    "The 2-Clause BSD License",
    "The Apache License, Version 2.0",
    "The Apache Software License, Version 2.0",
    "The MIT License",
    "The MIT License (MIT)"

Comparing this list to https://apache.org/legal/resolved.html, the only questions I see would be

  • Category B licenses - ok if labelled
    • CDDL
    • Eclipse
    • Mozilla
  • EDL -- what's that? But it's used by jakarta.activation so probably every module?
  • GNU LGPL - prohibited in the apache document, but almost every dependency licensed under LGPL is also separately licensed under something above. The exception is org.z3950.zing:cql-java:1.13

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDL -- what's that?

Please click on the link I've posted on 2024-12-16.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Julian, that's a great analysis, thank you.

* [X] Installation documentation is included
* -_note: read more at https://github.com/folio-org/mod-search/blob/master/README.md_
* [X] Personal data form is completed, accurate, and provided as `PERSONAL_DATA_DISCLOSURE.md` file
* [X] Sensitive and environment-specific information is not checked into git repository
- Postgres password in docker-compose.yml, but that's standard.
* [X] Module is written in a language and framework from the [officially supported technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) page[^1]
* [ ] Module only uses FOLIO interfaces already provided by previously accepted modules _e.g. a UI module cannot be accepted that relies on an interface only provided by a back end module that hasn't been accepted yet_
- Three new library dependencies, these are also in the TCR process currently:
- lib-linked-data-dictionary
- lib-linked-data-fingerprint
- lib-linked-data-marc4ld
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A library dependency is not a FOLIO interface.

A FOLIO interface is a set of API endpoints. Examples: https://dev.folio.org/reference/api/endpoints/

Required and optional interfaces have to be declared in the "requires" and "optional" section of the module descriptor. Example: https://github.com/folio-org/mod-linked-data/blob/master/descriptors/ModuleDescriptor-template.json#L163-L176

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fair point, you are correct. But I think the intent of this criterion and the one directly above (about Officially Supported Technologies) is to avoid approving things with new, unapproved dependencies. Which the dev team acknowledges in their submission that these libraries are.

Dependent on: lib-linked-data-marc4ld, lib-linked-data-fingerprint and lib-linked-data-dictionary

Perhaps we need to tweak this criterion (or else to add a new criterion) about dependent libraries also having to be already approved.

In practice, we tend to review backend modules before paired front-end modules to avoid this specific timing situation. I think we should just review backend libraries before paired backend modules, and then we avoid the same issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A module can use any software library, I don't see why only approved FOLIO libraries can be used whereas non-FOLIO libraries can always be used. Software developers can simply fork a non-approved FOLIO library to circumvent this restriction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A module can use any software library, I don't see why only approved FOLIO libraries can be used whereas non-FOLIO libraries can always be used. Software developers can simply fork a non-approved FOLIO library to circumvent this restriction.

This is a fair criticism of our current process, but I'm not sure what solution you are suggesting. If we reverse the decision to review & approve FOLIO libraries, then we incentivize developers to put anything controversial into a library. So I don't think we should do that. If you are suggesting that we treat non-FOLIO libraries like any other third-party tech to consider (as in the OST process), that's going to be a big conversation :-) I'm guessing you have a better suggestion.

If you would like to do a suggested edit for TCR process improvements below, I would be happy to merge it into the PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A module can use any software library, I don't see why only approved FOLIO libraries can be used whereas non-FOLIO libraries can always be used

My understanding of this is slightly different

The officially supported technologies page includes non-FOLIO libraries. This was intended to reflect decisions where the community had chosen a particular tool, e.g. JUnit for java based unit testing, and it was expected that folks should choose to use that for their FOLIO modules

This was a compromise between expecting development teams to seek approval for every new third party dependency and trying to constrain the choices to a degree. Much like many areas of FOLIO policies, it relies on good faith actions by the folks involved

* [X] Module gracefully handles the absence of third party systems or related configuration
- Works without Kafka. Otherwise, there are no third-party configs that the module should reasonably survive being missing.
* [X] Sonarqube hasn't identified any security issues, major code smells or excessive (>3%) duplication (6); and any disabled or intentionally ignored rules/recommendations are reasonably justified.
* See [Rule Customization](https://dev.folio.org/guides/code-analysis/#rule-customization) details.
julianladisch marked this conversation as resolved.
Show resolved Hide resolved
* [X] Uses [officially supported](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) build tools[^1]
* [X] Unit tests have 80% coverage or greater, and are based on [officially supported technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies)[^1]
julianladisch marked this conversation as resolved.
Show resolved Hide resolved

## Backend
* [X] Module's repository includes a compliant Module Descriptor
* -_note: read more at https://github.com/folio-org/okapi/blob/master/okapi-core/src/main/raml/ModuleDescriptor.json_
* [X] Module includes executable implementations of all endpoints in the provides section of the Module Descriptor
* [X] Environment vars are documented in the ModuleDescriptor
* -_note: read more at [https://wiki.folio.org/pages/viewpage.action?pageId=65110683](https://wiki.folio.org/pages/viewpage.action?pageId=65110683)_
* [X] If a module provides interfaces intended to be consumed by other FOLIO Modules, they must be defined in the Module Descriptor "provides" section, and must conform to FOLIO [interface naming conventions](https://dev.folio.org/guidelines/naming-conventions/#interfaces).
* [X] All API endpoints are documented in OpenAPI.
* [X] All API endpoints protected with appropriate permissions as per the following guidelines and recommendations, e.g. avoid using `*.all` permissions, all necessary module permissions are assigned, etc.
* -_note: read more at https://dev.folio.org/guidelines/naming-conventions/ and https://wiki.folio.org/display/DD/Permission+Set+Guidelines_
* [X] Module provides reference data (if applicable), e.g. if there is a controlled vocabulary where the module requires at least one value
- No reference data.
* [X] If provided, integration (API) tests must be written in an [officially supported technology](https://wiki.folio.org/display/TC/Officially+Supported+Technologies)[^1]
* -_note: while it's strongly recommended that modules implement integration tests, it's not a requirement_
* -_note: these tests are defined in https://github.com/folio-org/folio-integration-tests_
* [X] Data is segregated by tenant at the storage layer
- Yes, but there is some API cruft that a Jira is addressing: https://folio-org.atlassian.net/browse/MODLD-604
* [X] The module doesn't access data in DB schemas other than its own and public.
* [X] Any dependencies, other than on defined interfaces, are declared in the README.md.
maccabeelevine marked this conversation as resolved.
Show resolved Hide resolved
* [X] The module responds with a tenant's content based on x-okapi-tenant header
* [X] Standard GET `/admin/health` endpoint returning a 200 response
* -_note: read more at https://wiki.folio.org/display/DD/Back+End+Module+Health+Check+Protocol_
* [X] High Availability (HA) compliant
* Possible red flags:
* Connection affinity / sticky sessions / etc. are used
* Local container storage is used
* Services are stateful
* [X] Module only uses infrastructure / platform technologies on the [officially supported technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) list.[^1]
* _e.g. PostgreSQL, ElasticSearch, etc._

## TCR Process Improvements
[_Please include here any suggestions that you feel might improve the TCR Processes._]

[^1]: Refer to the [Officially Supported Technologies](https://wiki.folio.org/display/TC/Officially+Supported+Technologies) page for the most recent ACTIVE release.