move over most trivial services to backend-defaults #24724

freben · 2024-05-10T14:20:29Z

This moves over most of the remaining core service factories/implementations to backend-defaults.

NOTE that this is on top of the freben/tasks-plugin-api branch (#24563), adding to what it started.

There are still some slightly less trivial services to move, such as auth and logging.

Small point of interest: BackendInitializer now duck-types the lifecycle detection instead of doing instanceof. The reason I changed this is that I was afraid that the initializer would lead to issues because one or the other of the now two implementations of the services got passed to it.

camilaibs

Thank you so much @freben for opening this pull request. Left a few comments below 🙂

camilaibs · 2024-05-13T07:30:36Z

packages/backend-defaults/src/entrypoints/cache/cacheServiceFactory.ts

+ * limitations under the License.
+ */
+
+import { CacheManager } from '@backstage/backend-common';


Should the CacheManager be extracted to this package as well?

Opening this up for discussion! Happy to expose them from here if we want. Just gotta make sure we're happy to do so and whether it is better to wait for some service extension points to appear instead.

camilaibs · 2024-05-13T07:31:21Z

packages/backend-defaults/src/entrypoints/urlReader/urlReaderServiceFactory.ts

+ * limitations under the License.
+ */
+
+import { UrlReaders } from '@backstage/backend-common';


Should the UrlReaders be extracted to this package as well?

camilaibs · 2024-05-13T07:31:44Z

packages/backend-defaults/src/entrypoints/database/databaseServiceFactory.ts

+ * limitations under the License.
+ */
+
+import { DatabaseManager } from '@backstage/backend-common';


Should the DatabaseManager be extracted to this package as well?

camilaibs · 2024-05-13T07:37:40Z

packages/backend-defaults/package.json

@@ -20,22 +20,55 @@
  "license": "Apache-2.0",
  "exports": {
    ".": "./src/index.ts",
+    "./cache": "./src/entrypoints/cache/index.ts",


Does it make sense to create a codemod to assist adopters with updating their imports?

I could try!

camilaibs · 2024-05-13T07:40:34Z

packages/backend-defaults/src/entrypoints/cache/cacheServiceFactory.ts

+
+import { CacheManager } from '@backstage/backend-common';
+import {
+  coreServices,


I am more and more inclined to suggest renaming coreServices to defaultServices... Are there any conceptual differences between core and default services? It may also make sense to move the definitions to this package if we rename to defaultServices.

Oh that's a bit of a bigger discussion I guess. We've called them core services all the time - the ones that are in backend-plugin-api. That's where the nomenclature comes from, irrespective of if backend-defaults also exists as a concept or not. They are the "core set of interfaces that you can consume". The default implementations of those core interfaces live in backend-defaults, but you may bring your own.

Is it right to say core services are the ones the backend application can't function without? Without a clear definition of what makes a service "core", I think it would simplify things if we considered them all as services with standard implementations since they're essential to a minimal backend. What about root services? Are they all core? Maybe it's just me, but do you remember other people asking about this or making confusion about core, root, and plugins services 🤔 ?

Is it right to say core services are the ones the backend application can't function without?

They are the ones that backend-plugin-api expose. No more, no less. They are owned by the core maintainers, and we guarantee that they have sensible definitions out of the box. It's "our sdk" so to speak. I don't think the definition is more complex than that. :)

Maybe it's more useful to think in terms of, what do we move to backend-plugin-api and what do we leave outside? We generally put things in there that we feel are so foundational, and useful for so many plugins, that we want to make it part of the central guaranteed offering. Events are such a thing that started outside as a plugin, but then got considered for "adoption" as a foundational piece, because it really is a foundation that others should be encouraged to use and share information between each other with.

Another thing that is special about many (but not all) core services is that they really don't have anywhere else to live. We don't have a logging plugin that can hold the logger.

Without a clear definition of what makes a service "core", I think it would simplify things if we considered them all as services with standard implementations since they're essential to a minimal backend.

They're not though. It's just a toolbox. The things that we want to offer since we want to encourage people to build upon those primitives instead of reinventing wheels all over. Arguably not even config and logger are "essential", you just use them if you need them and we encourage you to try to leverage that instead of env vars and console logs, because it makes your plugin a better citizen.

What about root services? Are they all core?

No, being root or plugin scoped is a separate orthogonal concern. The service itself chooses what makes sense in its own situation. That only deals with the "lifecycle" / instantiation of the service.

Maybe it's just me, but do you remember other people asking about this or making confusion about core, root, and plugins services 🤔 ?

Hm not sure. The root/plugin thing I don't think I've heard. Figuring out where your deps live, yeah maybe? It's easy to find coreServices, people quickly pick up on that and enjoy the IDE completion helping them find what's available. But then, why isn't the catalog client in there for example? Sure, that gets a bit philosophical then, why is that not considered core but some others are? I kinda have an answer, but it's not always clear from the outside.

camilaibs · 2024-05-13T07:43:23Z

packages/backend-defaults/src/entrypoints/rootLifecycle/rootLifecycleServiceFactory.test.ts

+ * limitations under the License.
+ */
+
+import { getVoidLogger } from '@backstage/backend-common';


I'm moving this helper to the backend-plugin-api package, but I would like to confirm if moving to backend-default makes more sense.

That sounds more like a testUtils thing.

Should we use the mockServices.logger.mock() instead? This question just came to my mind now.

packages/backend-defaults/src/entrypoints/permissions/permissionsServiceFactory.ts

packages/backend-app-api/src/wiring/BackendInitializer.ts

camilaibs · 2024-05-13T07:49:08Z

packages/backend-app-api/src/services/implementations/cache/cacheServiceFactory.ts

@@ -20,7 +20,10 @@ import {
  createServiceFactory,
 } from '@backstage/backend-plugin-api';

-/** @public */
+/**


Should we move the deprecated files to a deprecated folder? Just for making it easy to cleanup later?

Ah yeah that could really help!

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

camilaibs

Approving as the comments left are optional and could be addressed in a follow-up pull request. Thank you again 🙌🏻

Rugvip

Awesome stuff! 👏 Let's

Rugvip · 2024-05-17T11:19:47Z

packages/backend-app-api/src/wiring/BackendInitializer.ts

+    const service = lifecycleService as any;
+    if (service && typeof service.startup === 'function') {
+      return service;
    }


github-actions · 2024-05-17T11:34:36Z

Uffizzi Cluster pr-24724 was deleted.

freben requested review from backstage-service and a team as code owners May 10, 2024 14:20

freben requested review from benjdlambert and vinzscam and removed request for a team May 10, 2024 14:20

freben force-pushed the freben/more-default-moves branch from ca3b43f to 710da83 Compare May 10, 2024 15:06

freben requested a review from Rugvip May 10, 2024 15:07

camilaibs reviewed May 13, 2024

View reviewed changes

camilaibs linked an issue May 13, 2024 that may be closed by this pull request

Extract all service implementations from backend-common to the backend-defaults package #24736

Closed

freben force-pushed the freben/tasks-plugin-api branch from 11c5cbc to ddfd660 Compare May 16, 2024 08:12

move over most trivial services to backend-defaults

b0210ec

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

freben force-pushed the freben/more-default-moves branch from 710da83 to b0210ec Compare May 16, 2024 08:27

camilaibs approved these changes May 16, 2024

View reviewed changes

Base automatically changed from freben/tasks-plugin-api to master May 17, 2024 11:07

Rugvip approved these changes May 17, 2024

View reviewed changes

freben merged commit 26cf95a into master May 17, 2024
31 checks passed

freben deleted the freben/more-default-moves branch May 17, 2024 11:33

This was referenced May 21, 2024

continued move of core services to backend-defaults #24848

Merged

continued continued move of core services to backend-defaults #24849

Closed

prefixed url reader types with the service name #24853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move over most trivial services to backend-defaults #24724

move over most trivial services to backend-defaults #24724

freben commented May 10, 2024 •

edited by camilaibs

camilaibs left a comment

camilaibs May 13, 2024

freben May 13, 2024

camilaibs May 13, 2024

camilaibs May 13, 2024

camilaibs May 13, 2024

freben May 13, 2024

camilaibs May 13, 2024

freben May 13, 2024

camilaibs May 14, 2024

freben May 14, 2024

camilaibs May 13, 2024

freben May 13, 2024

camilaibs May 14, 2024

freben May 14, 2024

camilaibs May 13, 2024

freben May 13, 2024

camilaibs left a comment

Rugvip left a comment

Rugvip May 17, 2024

github-actions bot commented May 17, 2024

move over most trivial services to backend-defaults #24724

move over most trivial services to backend-defaults #24724

Conversation

freben commented May 10, 2024 • edited by camilaibs

camilaibs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilaibs left a comment

Choose a reason for hiding this comment

Rugvip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 17, 2024

freben commented May 10, 2024 •

edited by camilaibs