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

Restructure Remote Service doc for Java to include Service Binding based services #732

Merged
merged 44 commits into from
Apr 2, 2024

Conversation

StefanHenke
Copy link
Contributor

@StefanHenke StefanHenke commented Mar 8, 2024

This is a first draft of the structure for review. Does not yet contain all content.

The idea is to get some initial feedback on the new structure.

Preview build: https://capire-binding-based-remote-service.cfapps.sap.hana.ondemand.com/docs/java/cqn-services/remote-services

java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved

[Learn more about destinations in the **SAP Cloud SDK documentation**.](https://sap.github.io/cloud-sdk/docs/java/features/connectivity/sdk-connectivity-destination-service){.learn-more}

As a variant to the described scenario, it is possible to enable multi-tenant CAP applications to lookup the BTP Destination from the subaccount of the subscriber tenant instead of the subaccount in which the CAP application is deployed. This allows you to provide tenant-specific callbacks as extension use cases.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the default behaviour already, no explicit configuration is required for this (except the destination dependency in SaaS). Maybe we should rather turn the logic around and describe how to disable this, by setting the right retrieval strategy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have adapted it again. It now does not feel as a variant anymore. But anyhow, after thinking again about the s4 scenario, I think this is actually always required to call into the subscriber tenant. Defining a destination to a s4 system in the provider account feels strange. S4 systems are not multi-tenant aware and, hence, are always dedicated for a specific customer with customer-specific credentials.

java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Spelling Mistakes

  • ./java/cqn-services/remote-services.md:117:21 Unknown word "Configuratiion"
  • ./java/cqn-services/remote-services.md:135:189 Unknown word "xsuua"

Generally, for each spelling mistake there are 2 ways to fix it:

  1. Fix the spelling mistake and commit it.
  2. The word is incorrectly reported as misspelled → put the word on the project-words.txt list, located in the root project directory.

java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Spelling Mistakes

  • ./java/cqn-services/remote-services.md:137:189 Unknown word "xsuua"

Generally, for each spelling mistake there are 2 ways to fix it:

  1. Fix the spelling mistake and commit it.
  2. The word is incorrectly reported as misspelled → put the word on the project-words.txt list, located in the root project directory.

java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
StefanHenke and others added 3 commits March 20, 2024 11:41
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Spelling Mistakes

  • ./java/cqn-services/remote-services.md:120:4 Unknown word "constrast"

Generally, for each spelling mistake there are 2 ways to fix it:

  1. Fix the spelling mistake and commit it.
  2. The word is incorrectly reported as misspelled → put the word on the project-words.txt list, located in the root project directory.

java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
Comment on lines +1674 to +1675
| OAuth2ClientCredentials | [code only](../java/cqn-services/remote-services#register-destinations) | <X/> |
| UserTokenAuthentication | [code only](../java/cqn-services/remote-services#register-destinations) | <X/> |
Copy link
Contributor

Choose a reason for hiding this comment

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

code only is slightly misleading here now. I think we should make clear that this also works via configuration now (maybe simply <Y/> and link to our docs).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I have linked to our new binding-based approach. I guess this should be the preferred way over programmatically registering destinations

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this guide needs to be adapted overall. It still deprecated property structures in a lot of areas, due to our latest changes. Also we should at least mention somewhere that in Java the new binding-based destinations are available (see also concerns below)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I adapted all configurations. The whole guide focuses a lot on destinations, I was not aware of that guide before. Not sure if we need to adapt the guide on a second step to describe the binding-based approach more prominently.

java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
java/cqn-services/remote-services.md Outdated Show resolved Hide resolved
@renejeglinsky renejeglinsky merged commit 6c402d1 into main Apr 2, 2024
3 of 4 checks passed
@renejeglinsky renejeglinsky deleted the binding-based-remote-service branch April 2, 2024 14:34
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