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.

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

UID-184 Add okapi interface to dependencies. #452

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

MikeTaylor
Copy link
Contributor

We have long depended on this.

Fixes UID-184.

We have long depended on this.

Fixes UID-184.
@MikeTaylor MikeTaylor requested a review from zburke January 28, 2025 13:42
Copy link
Member

@zburke zburke left a comment

Choose a reason for hiding this comment

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

LGTM. Please include UID-184 at the start of the commit subject; it makes parsing the commit log much easier and is especially useful for release scripts that rely on finding Jira slugs here in order to automatically update Jira's "fix version" field when releases are published from GitHub.

@MikeTaylor
Copy link
Contributor Author

LGTM. Please include UID-184 at the start of the commit subject; it makes parsing the commit log much easier and is especially useful for release scripts that rely on finding Jira slugs here in order to automatically update Jira's "fix version" field when releases are published from GitHub.

Sure thing.

@MikeTaylor MikeTaylor merged commit ef0204c into master Jan 28, 2025
14 checks passed
@MikeTaylor MikeTaylor deleted the UID-184--add-okapi-interface-dependency branch January 28, 2025 14:27
@zburke zburke changed the title Add okapi interface to dependencies. UID-184 Add okapi interface to dependencies. Jan 28, 2025
@adamdickmeiss
Copy link

adamdickmeiss commented Jan 28, 2025

Guys. You are too quick. If en Eureka mode, fine. But okapi does NOT offer the okapi interface (not yet). See folio-org/okapi#1370

@MikeTaylor
Copy link
Contributor Author

Yet https://folio-snapshot.dev.folio.org/settings/developer/okapi-console?tab=interfaces reports that the okapi interface is provided by okapi-6.2.0-SNAPSHOT.

@adamdickmeiss
Copy link

adamdickmeiss commented Jan 28, 2025

Yet https://folio-snapshot.dev.folio.org/settings/developer/okapi-console?tab=interfaces reports that the okapi interface is provided by okapi-6.2.0-SNAPSHOT.

It was provided as a special interfaceType: system which was NOT considered in dependency resolution, in essence not provided. As you can see there are TWO modules apparently offering the same interface. There should only be one of them (facade or okapi itself). Do you know the call the developer UI is using to show the interfaces ?

@MikeTaylor
Copy link
Contributor Author

It makes two calls:

The former only gives interface names and versions, so that can't be how it's associating the interface with Okapi. But for what it's worth, it's the very last interface (of 376) included in the response.

Must be something in the much bigger response to the latter.

@adamdickmeiss
Copy link

It makes sense the 2nd call shows all interfaces, regardless of type.

@MikeTaylor
Copy link
Contributor Author

So will be OK for this dependency to be included? Or will Okapi, when trying to enable ui-developer, see the okapi dependency and say "I've never heard of this Okapi thing, better reject the module"?

@adamdickmeiss
Copy link

So will be OK for this dependency to be included? Or will Okapi, when trying to enable ui-developer, see the okapi dependency and say "I've never heard of this Okapi thing, better reject the module"?

If operated by okapi it will never fail due to missing interface (okapi module provides okapi interface).

However, if there is another module that also offers an okapi interface , it will fail . And the admin will have to choose by explicitly telling what to enable (okapi or the other).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants