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

fix(cronjob expiry): added the missing environment value for portal #196

Conversation

dhiren-singh-007
Copy link
Contributor

@dhiren-singh-007 dhiren-singh-007 commented Jul 9, 2024

Description

Added environment value for PortalSettings in charts/ssi-credential-issuer/templates/cronjob-expiry-app.yaml
fixes #195
closes #195

Why

To run this project PortalSettings are required

Issue

Issue #195

Checklist

Please delete options that are not relevant.

  • I have followed the contributing guidelines
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally

@evegufy
Copy link
Contributor

evegufy commented Jul 9, 2024

Hi @dhiren-singh-007 thank you for the fix! Just letting you know that the review will take a couple of days as @Phil91 is on vacation this week and I'd like him to review as well.

I assume the config should also be added to the appsettings as we want maintain all config there as well, but let's wait for @Phil91 feedback

@dhiren-singh-007
Copy link
Contributor Author

@evegufy No problem . Thanks for the update .
Agree appsettings should be updated , I will wait for @Phil91 feedback.

Copy link
Member

@Phil91 Phil91 left a comment

Choose a reason for hiding this comment

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

hey @dhiren-singh-007 thanks for the adjustment. the changes look good to me. Could you please update the appsettings.json in the SsiCredentialIssuer.Expiry.App project to include the Portal section as well similar to

Copy link
Contributor

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

Unfortunately you merged the wrong branch (main instead of the release branch): could you please rebase to release branch so that only you commits are shown in the diff?

@dhiren-singh-007
Copy link
Contributor Author

Unfortunately you merged the wrong branch (main instead of the release branch): could you please rebase to release branch so that only you commits are shown in the diff?

Thanks @evegufy , Yeah this happens after syncing the Fork from Github UI . It looks like I should use commands to rebase .

@dhiren-singh-007
Copy link
Contributor Author

oh i got it now , So in forked repo Github UI shows a update branch option . So i thought it just sync the changes of the branch which exist only on upstream but it actually merged the main from upstream . Let me try to fix it else i will create a new PR

@evegufy evegufy requested a review from Phil91 July 16, 2024 13:49
@Phil91 Phil91 merged commit dc6b130 into eclipse-tractusx:release/v1.1.0-rc.1 Jul 16, 2024
10 checks passed
@dhiren-singh-007 dhiren-singh-007 deleted the bugfix/195-fix-portal-settings branch July 17, 2024 08:55
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.

3 participants