SAP-samples / btp-cap-multitenant-saas

Sample project that demonstrates how to setup a multitenant application for a Software-as-a-Service scenario, leveraging the Kyma and Cloud Foundry Runtimes of the SAP Business Technology Platform. Developers learn how to implement their own CAP (mtxs) based SaaS app including an SaaS API and integration with various essential SAP BTP service of...
Apache License 2.0
89 stars 42 forks source link

Deployment CF broken due to @sap/cds v8 renamed cds-serve => serve #53

Closed daniel-huser closed 2 months ago

daniel-huser commented 2 months ago

Hi!

I was trying to deploy the app to CF and noticed that the command in the mta.yaml to start the cap app, wont work anymore with CAP 8, as the file in @sap/cds/bin/ named cds-serve.js was renamed to serve.js.

Old: command: node ./node_modules/@sap/cds/bin/cds-serve New: command: node ./node_modules/@sap/cds/bin/serve

Just wanted to check if that is an issue you on my side or if this is an intentional change? Couldn't find any mention of that in the CAP 8 release notes.

Regards, Daniel

P.S: If I agree with my fix, I can also create a small PR in case you are busy. But I thought to open an issue first.

gregorwolf commented 2 months ago

Hi @daniel-huser ,

the best practice is described here: https://cap.cloud.sap/docs/node.js/cds-server#cli-command-cds-serve

alperdedeoglu commented 2 months ago

Hi @daniel-huser , Thank you very much for this contribution. Very happy to see that our repo gaining more enthusiastic people like yourself. We also discussed this just yesterday with @yevgentrukhin that this might cause an issue on CF deployment, and I agree that the solution you would propose would help in this case.

Will make sure that deployment works seamlessly again. Gonna update here with PR.

daniel-huser commented 2 months ago

Hi @daniel-huser ,

the best practice is described here: https://cap.cloud.sap/docs/node.js/cds-server#cli-command-cds-serve

Agreed, that using the cli is best practice, but I didn't want to challenge this as my first "issue" here :)

yevgentrukhin commented 2 months ago

I just tried on my copy of this tutorial and it works after removing 'command' from mta.yaml and relying on start up script in package.json with 'cds-serve' or 'npx cds-serve'. I hope this helps.

alperdedeoglu commented 2 months ago

Thanks @yevgentrukhin for suggestion and thanks @gregorwolf for addressing the best practice.

Thanks @daniel-huser for addressing the issue, please do not hesitate challenging the repo. Any feedback and contribution is very welcome.