Skip to content

Conversation

Akatuoro
Copy link
Contributor

@Akatuoro Akatuoro commented Oct 7, 2025

This somehow found it's way in... the build does not work with with-mtx-sidecar, the correct profile here is mtx-sidecar as also described in the node section.

Updating the sidecar package.json to what is currently generated (incl java profile).

@renejeglinsky
Copy link
Contributor

Thanks @Akatuoro ,
what is actually added by cds add multitenancy? The details box described what this command adds and if that is also wrong we need to check this with our colleagues.

@Akatuoro
Copy link
Contributor Author

Akatuoro commented Oct 8, 2025

@renejeglinsky , good point, but it's just the documentation. cds add multitenancy adds the correct profiles:

$ cds init bookshop-mt --add java,multitenancy
...
$ cd bookshop-mt
$ cat mtx/sidecar/package.json 
{
  "name": "bookshop-mt-mtx",
  "dependencies": {
    "@cap-js/hana": "^2",
    "@sap/cds": "^9",
    "@sap/cds-mtxs": "^3",
    "@sap/xssec": "^4",
    "express": "^4"
  },
  "devDependencies": {
    "@cap-js/sqlite": "^2"
  },
  "engines": {
    "node": ">=20"
  },
  "scripts": {
    "start": "cds-serve",
    "build": "cds build ../.. --for mtx-sidecar --production && npm ci --prefix gen"
  },
  "cds": {
    "profiles": [
      "mtx-sidecar",
      "java"
    ]
  }
}

@renejeglinsky
Copy link
Contributor

@swaldmann can you please explain when that has changed? I couldn't find anything relevant in the changelog and I assume the docs were right at one point in time, or?

@renejeglinsky
Copy link
Contributor

renejeglinsky commented Oct 8, 2025

Hi @Akatuoro ,

I think we're looking at different things. You're checking the sidecar package.json. But the docs is about the .cdsrc.json of the root project. So at both places it is as described, I think.

/bookshop-mt$ cat .cdsrc.json
{
  "cdsc": {
    "draftMessages": true
  },
  "profiles": [
    "with-mtx-sidecar",
    "java"
  ],
  "requires": {
    "[production]": {
      "multitenancy": true
    }
  }
}

@Akatuoro
Copy link
Contributor Author

Akatuoro commented Oct 8, 2025

You're right, this is the app config, not the sidecar... Both me and some colleagues missed that :D

I've reverted the original change and instead updated the sidecar package.json with what is currently generated. That was missing the java profile. Future goal would be that we don't need to provide the java profile because it is automatically propagated, but currently there can be errors if the profile is not given.
There's also the engine which is now included in the generated package.json, so I just pasted the whole generated json to be up to date.

@swaldmann
Copy link
Contributor

Could be a challenge to detect Java implicitly as the sidecar doesn't include the pom.xml or any other indication that it's running with a Java backend.

Copy link
Contributor

@renejeglinsky renejeglinsky left a comment

Choose a reason for hiding this comment

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

This details box should only show what is actually added by cds add multitenancy. In addition people might miss it here. Therefore we add this outside the details box.

@renejeglinsky
Copy link
Contributor

renejeglinsky commented Oct 9, 2025

@swaldmann Is this configuration preset the "profile": "java" we're about to add here?
image

If yes, I'd read it as 'this comes with the "with-mtx-sidecar" profile' and am wondering why it didn't work for @Akatuoro

@renejeglinsky
Copy link
Contributor

[...] updated the sidecar package.json with what is currently generated. That was missing the java profile. [...]

Thanks @Akatuoro , I then have misinterpreted your previous comment or wasn't aware of your scenario that you described now.

@renejeglinsky renejeglinsky merged commit 82936ea into main Oct 9, 2025
7 checks passed
@renejeglinsky renejeglinsky deleted the fix-sidecar-profiles branch October 9, 2025 07:10
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.

4 participants