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

Error handling vol. 2.0 #984

Open
przemkalit opened this issue Nov 22, 2024 · 8 comments · May be fixed by #1018
Open

Error handling vol. 2.0 #984

przemkalit opened this issue Nov 22, 2024 · 8 comments · May be fixed by #1018
Labels
enhancement New feature or request new New issue, this should be removed once reviewed

Comments

@przemkalit
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Allow managing errors during the import process—why can this be helpful?

Imagine you have plenty of JT, WF, and PRJ files to import, but you’re unsure which ones are valid. Sure, they were exported with success, but what if the import process fails at the first error? What I was proposing is to allow users to generate a list of problematic objects, attempt to fix them, and then relaunch the import process only for those invalid objects.

Describe the solution you'd like
There was solution introduced in #862, but was removed in #962.

Today I wanted to add this change to WF and PRJ but I notice that they were removed, and decided to create feature request, and ask you guys, maybe you have a better solution for this.

@przemkalit przemkalit added enhancement New feature or request new New issue, this should be removed once reviewed labels Nov 22, 2024
@djdanielsson
Copy link
Collaborator

the correct usage of config as code is that whatever is in the code is truth, and everything that is required for that config is in code. I think what you are describing sounds more of a backup strategy.

@Rocco83
Copy link

Rocco83 commented Nov 22, 2024

I would agree with @przemkalit
The feature is useful when only a "portion of the config as code" is sent as payload via API.

In my mind, at least 2 use cases exists to support the above statement

  1. huge config as code, avoiding in a very long time to apply the unchanged objects
  2. working with several team on different portion of AAP, as the repository must be "per team"

Thoughts?

@djdanielsson
Copy link
Collaborator

I am still thinking that everything should be in code, even if one team is managing just their repo all its dependencies should exist. another concern about this code is the added complexity which then increases the risk of breaking. @Tompage1994 @sean-m-sullivan thoughts?

@Rocco83
Copy link

Rocco83 commented Nov 22, 2024

just to clarify,
I totally agree that everything must be in code - meaning in a given repository, one or multiple.
Still, this missing dependencies may happen unless the full payload is shipped - which is not doable, that's too slow for a huge environment.

As a step further, there can be even the ability to include the missing resources automatically - but this is much more complicated.

BTW, if there is any other idea on how to achieve the goal of error handing, while keeping the payload as small as possible - to have a fast run - i'll be glad to hear&discuss about it (in another thread?)

My 2 cents,

@Tompage1994
Copy link
Collaborator

Personally, I think this is a valid addition to this code base. The key reason it was taken out was that it hadn't been completed across all relevant endpoints and led to incosistencies (plus there was a bug which we didn't want to resolve at the time).

In my view this is an addition that doesn't affect the single source of truth, but simply adds to reliability and ease of execution. To me a major point of this was that it would continue to apply the config in full and then essentially warn about what is missing which to me will give you more consistency with what is in code than exists today.

So to me, I think we should re-add it in, but it should be thoroughly tested and replicated for all relevant roles. I think what may have been miossing before as well was something to mark a play as failed if any errors did occur.

@djdanielsson
Copy link
Collaborator

I agree if we do add it back it needs to be everywhere for constancy, I still am concerned about adding more complexity but it is what it is. @Rocco83 one of my long term ideas/goals since EDA was announced was to utilize it to read the git web hook and determine what changes were made and do just those API calls, idk if we could really get down to specific objects but at least object types should be possible. If you want to talk more about it, feel free to open a different issue to track that

@przemkalit
Copy link
Contributor Author

I want to clarify one thing, what was the bug that you discovered?

@djdanielsson
Copy link
Collaborator

@Tompage1994 ?

@przemkalit przemkalit linked a pull request Dec 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new New issue, this should be removed once reviewed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants