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

[feat|fix] : SDE backend : Hotfix 3.2 with pcf submodel : rel-2.3.0 #68

Closed

Conversation

sachinargade123
Copy link
Contributor

@sachinargade123 sachinargade123 commented Nov 21, 2023

Description

Added

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

Fixed

  • DT use refactor and PCF issue fix.
  • Correct dataplane endpoint for digital twin.
  • DSP endpoint path for digital-twin edc url.
  • trivy worklow fix.

Pre-review checks

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

Copy link
Contributor

@almadigabor almadigabor left a comment

Choose a reason for hiding this comment

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

Can you please address my question below?

INSTALL.md Outdated
- JDK18
- Postgres 11.9.13
- JDK17
- Postgres 13.2.11
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you specify Postgres v13 when in your helm chart you use chart version 11.x.x which potentially means Postgres v15?

Copy link
Contributor

@almadigabor almadigabor Nov 21, 2023

Choose a reason for hiding this comment

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

Sorry, I was wrong, Bitnami's Postgresql chart version 11.x.x means Postgresql v14. Although based on this TRG you should have already moved/upgraded to Postgres v15 which is chart version 12.x.x.

Copy link
Contributor

@adityagajbhiye9 adityagajbhiye9 Nov 22, 2023

Choose a reason for hiding this comment

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

We will be using Postgres v16. Can we use Postgres v13.X.X.

Copy link
Contributor

Choose a reason for hiding this comment

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

Postgress upgraded to v15.

@adityagajbhiye9
Copy link
Contributor

adityagajbhiye9 commented Nov 22, 2023

@almadigabor Changes made as per suggested.
#69 is fixed. Changed in run cmd.

@adityagajbhiye9
Copy link
Contributor

Fixed
#71 Notice.md updated.

@ChetanT-System
Copy link
Contributor

Fixed
#70
Please review @sachinargade123

@MehranRoshandel
Copy link

@almadigabor, can you please help us to merge this PR by today?

@hzierer
Copy link
Contributor

hzierer commented Nov 24, 2023

can you please update the dependencies file? I also open a PR for that 3 days ago.

@sachinargade123
Copy link
Contributor Author

sachinargade123 commented Nov 24, 2023

@hzierer,

We have already Updated Dependencies file.

Thanks for created PR, We have checked it but that is not sufficient, our SDE backend is multi maven model project so it should scan all project and autogenerate dependency file. We will take look and generate DEPEDENCY file automatically.

@adityagajbhiye9
Copy link
Contributor

@almadigabor Can we merge the PR?

@hzierer
Copy link
Contributor

hzierer commented Nov 24, 2023

I split the old PR in two separate ones and fixed the dash tool to include the testing scope:
#72
#73

@MehranRoshandel
Copy link

So it is reviewed and can be merged now. Correct?

@sachinargade123
Copy link
Contributor Author

@hzierer
We had already included this #73 changes in here sde-core maven model where actual application jar get build . May be we don't require this changes.

@hzierer
Copy link
Contributor

hzierer commented Nov 24, 2023

@hzierer We had already included this #73 changes in here sde-core maven model where actual application jar get build . May be we don't require this changes.

yeah ok, it might be obsolete then. just saw that you are creating two jars and wanted to make sure that theese documents are part of all. But I assume then, that only the one in sde-core is distributed to dockerhub

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.

This PR is too big as followed our guidelines
https://www.eclipse.org/projects/handbook/#ip-project-content

@sachinargade123
Copy link
Contributor Author

Ok sure we are closing this PR and we will open new PRs.

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.

7 participants