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.

Sign up for GitHub

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

Jump to bottom

[feat|fix] : SDE backend : hot fix for 3.2 with pcf version #75

Merged
merged 5 commits into from
Nov 28, 2023
Merged

[feat|fix] : SDE backend : hot fix for 3.2 with pcf version #75

merged 5 commits into from
Nov 28, 2023

Conversation

sachinargade123
Copy link
Contributor

@sachinargade123 sachinargade123 commented Nov 27, 2023 •
edited
Loading

please close PR #74 this before this

Description

Added

  • Added oauth security for sde public api.
  • BPN url add API path.

Fixed

  • Correct dataplane endpoint for digital twin.
  • DSP endpoint path for digital-twin edc url.

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@adityagajbhiye9
Change log update
9039a3c
@adityagajbhiye9
Merge branch 'hotfix_3.2_with_pcf_part_1' of https://github.com/caten…
1daff13 
…ax-ng/tx-managed-simple-data-exchanger-backend into hotfix_3.2_with_pcf_part_2
@adityagajbhiye9
Docker image update
d3bf0be 
- Docker image update.
- legal info updated for image.
- change log update.
- Install. md update.
@adityagajbhiye9 adityagajbhiye9 mentioned this pull request Nov 28, 2023
Merged
2 tasks
@adityagajbhiye9
Copy link
Contributor

@FaGru3n Please merge this PR after #74 PR.

@FaGru3n
Copy link
Contributor

FaGru3n commented Nov 28, 2023

@adityagajbhiye9 could you please raise a issue against us as to become a least a contributor to the Ecplise Tractus-X GitHub Organisation https://github.com/eclipse-tractusx/sig-infra/issues/new?assignees=&labels=support&projects=&template=support-add-project-contributor.md&title=New+Tractus-X+project+contributor

@FaGru3n
Copy link
Contributor

FaGru3n commented Nov 28, 2023

@sachinargade123 could you please give me the information who is also in your team to validate the changes?

@FaGru3n FaGru3n requested a review from hzierer November 28, 2023 09:22
@adityagajbhiye9
Copy link
Contributor

@adityagajbhiye9 could you please raise a issue against us as to become a least a contributor to the Ecplise Tractus-X GitHub Organisation https://github.com/eclipse-tractusx/sig-infra/issues/new?assignees=&labels=support&projects=&template=support-add-project-contributor.md&title=New+Tractus-X+project+contributor

Sure, I will raise a issue.

@FaGru3n
Copy link
Contributor

FaGru3n commented Nov 28, 2023

@adityagajbhiye9 could you please raise a issue against us as to become a least a contributor to the Ecplise Tractus-X GitHub Organisation https://github.com/eclipse-tractusx/sig-infra/issues/new?assignees=&labels=support&projects=&template=support-add-project-contributor.md&title=New+Tractus-X+project+contributor

Sure, I will raise a issue.

if i want to merge your PR ECA Check will fail, because you directly committed on @sachinargade123 branch... unless your a contributor to this project

please be also aware of
https://eclipse-tractusx.github.io/docs/oss/how-to-contribute
https://eclipse-tractusx.github.io/docs/oss/getting-started

@FaGru3n FaGru3n requested a review from dvasunin November 28, 2023 09:30
@FaGru3n
Copy link
Contributor

FaGru3n commented Nov 28, 2023

@dvasunin could you please verify the changes from @sachinargade123 and @adityagajbhiye9

@adityagajbhiye9
Copy link
Contributor

@FaGru3n I have raised issue.
eclipse-tractusx/sig-infra#381

dvasunin
dvasunin reviewed Nov 28, 2023
View reviewed changes
...ins/src/main/java/org/eclipse/tractusx/sde/digitaltwins/facilitator/DigitalTwinsUtility.java

@Value(value = "${edc.hostname}${edc.dataplane.endpointpath:/api/public}")
public String digitalTwinEdcDataplaneEndpoint;

ObjectMapper mapper = new ObjectMapper();
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: consider to make it private final

dvasunin
dvasunin approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@dvasunin dvasunin left a comment

Choose a reason for hiding this comment

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

LGTM

adityagajbhiye9
adityagajbhiye9 approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@adityagajbhiye9 adityagajbhiye9 left a comment

Choose a reason for hiding this comment

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

LGTM

@adityagajbhiye9
Copy link
Contributor

@FaGru3n I have reviewed and approved.

@ChetanT-System
Copy link
Contributor

ChetanT-System commented Nov 28, 2023 •
edited
Loading

@hzierer Please review and approve the PR
@FaGru3n Please merge merge the PR once approve !
Once merged this PR, We have to create one more PR(part_3) as part of PR split.

@FaGru3n
Copy link
Contributor

FaGru3n commented Nov 28, 2023 •
edited
Loading

@hzierer Please review and approve the PR @FaGru3n Please merge merge the PR once approve ! Once merged this PR, We have to create one more PR(part_3) as part of PR split.

Hi @ChetanT-System thanks for mentioning, but why the pressure to me to merge? Only following our Tractus-X Release Guidlines and when there is no committer in your team, would be good if there were some member. That the upstream branch can updated regularly.

see also... eclipse-tractusx/managed-simple-data-exchanger-frontend#45 (comment)

FaGru3n
FaGru3n approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@FaGru3n FaGru3n left a comment

Choose a reason for hiding this comment

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

LGTM - content can not verified structure of the PR seems good.

hzierer
hzierer approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@hzierer hzierer left a comment

Choose a reason for hiding this comment

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

Can't check the changes to java code. Rest LGTM

@FaGru3n FaGru3n merged commit 9cd4417 into eclipse-tractusx:main Nov 28, 2023
2 checks passed
@FaGru3n FaGru3n deleted the hotfix_3.2_with_pcf_part_2 branch November 28, 2023 15:35
@ChetanT-System
Copy link
Contributor

Hi @FaGru3n Thank you for your support ! I'm not pressuring to you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adityagajbhiye9 adityagajbhiye9 adityagajbhiye9 approved these changes

@FaGru3n FaGru3n FaGru3n approved these changes

@hzierer hzierer hzierer approved these changes

@dvasunin dvasunin dvasunin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sachinargade123 @adityagajbhiye9 @FaGru3n @ChetanT-System @dvasunin @hzierer