feat: revisions info reporting with multi-sourced apps support #333
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oleksandr-codefresh
force-pushed
the
CR-23842-revisions-info-reporting
branch
from
1113941 to
0b50690
Compare
…rt multisourced apps
oleksandr-codefresh
force-pushed
the
CR-23842-revisions-info-reporting
branch
from
0b50690 to
37f5a05
Compare
…"app.meta.revisions-metadata"
…R-23842-revisions-info-reporting
…tions - app.meta.revisions-metadata
…etadata, report only revision in case of helm chart
pasha-codefresh
requested changes
Sep 27, 2024
| } | ||
| } | ||
|
|
||
| if a.Source != nil { // single source app |
There was a problem hiding this comment.
Can you please just check if app has source and return this source, otherwise execute logic after.
It is impossible that app will not have source at all
pasha-codefresh
requested changes
Sep 27, 2024
event_reporter/utils/app_revision.go
Outdated
| // for monorepo support: list with revisions where actual changes to source directory were committed | ||
| func GetOperationChangeRevisions(a *appv1.Application) []string { | ||
| var revisions []string | ||
| if a != nil { |
There was a problem hiding this comment.
could you please check if app is nil then return empty array, it will not have so much embedded conditions
event_reporter/utils/app_revision.go
Outdated
| @@ -46,6 +63,64 @@ func GetOperationRevision(a *appv1.Application) string { | |||
| return revision | |||
| } | |||
|
|
|||
| func GetOperationSyncRevisions(a *appv1.Application) []string { | |||
| var revisions []string | |||
| if a != nil { | |||
event_reporter/utils/app_revision.go
Outdated
| Revisions: a.Status.OperationState.Operation.Sync.ChangeRevisions, | ||
| }) | ||
| } | ||
| } else if a.Operation != nil && a.Operation.Sync != nil { |
There was a problem hiding this comment.
I think we will rid of idea to put it on operation level, we will just persist it on status
There was a problem hiding this comment.
it's just as a fallback so can leave ATM
oleksandr-codefresh
force-pushed
the
CR-23842-revisions-info-reporting
branch
from
17a0ef3 to
893fbcd
Compare
oleksandr-codefresh
force-pushed
the
CR-23842-revisions-info-reporting
branch
from
893fbcd to
9bc3265
Compare
…R-23842-revisions-info-reporting
…esh-io/argo-cd into CR-23842-revisions-info-reporting
pasha-codefresh
requested changes
Sep 30, 2024
|
|
||
| for idx, revision := range revisions { | ||
| // report just revision for helm sources | ||
| if (a.Spec.HasMultipleSources() && a.Spec.Sources[idx].IsHelm()) || (a.Spec.Source != nil && a.Spec.Source.IsHelm()) { |
There was a problem hiding this comment.
Please move this condition into dedicated types . Make it a.Spec.IsHelm(idx)
| if err == nil && operationSyncRevisionsMetadata != nil { | ||
| result.SyncRevisions = operationSyncRevisionsMetadata | ||
| } | ||
| // latest revision of repository where changes to app resource were actually made; empty if no changeRevisionі present |
| u = utils.AddCommitDetailsToLabels(u, originalAppRevisionMetadata) | ||
| if originalAppRevisionsMetadata != nil { | ||
| u = utils.AddCommitsDetailsToAnnotations(u, originalAppRevisionsMetadata) | ||
| if originalApplication != nil { |
event_reporter/utils/app_revision.go
Outdated
| @@ -46,6 +63,70 @@ func GetOperationRevision(a *appv1.Application) string { | |||
| return revision | |||
| } | |||
|
|
|||
| func GetOperationSyncRevisions(a *appv1.Application) []string { | |||
| var revisions []string | |||
There was a problem hiding this comment.
this is redundant variable, you can remove it
… func a.Spec.IsHelmSource(idx), removed legacy code
…esh-io/argo-cd into CR-23842-revisions-info-reporting
…DetailsToAnnotations, AddCommitsDetailsToAppAnnotations, getRevisions, getOperationSyncRevisions
…ionServiceClient.RevisionMetadata in order to properly support multisourced apps
oleksandr-codefresh
force-pushed
the
CR-23842-revisions-info-reporting
branch
from
c3aa11f to
09fddac
Compare
…R-23842-revisions-info-reporting # Conflicts: # changelog/CHANGELOG.md
pasha-codefresh
approved these changes
Oct 2, 2024
…s to labels as new runtimes should work on old on-prem versions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist: