Simplify the attributes sent to the entity_add handler #4517
Conversation
|
@blkt would you mind reviewing this? You probably know the code better than I |
| if repoName, ok = event.Entity["repoName"].(string); !ok { | ||
| return errors.New("invalid repo name") | ||
| if entName, ok = event.Entity[properties.PropertyName].(string); !ok { | ||
| return fmt.Errorf("invalid or unset entity name: %s", entName) |
There was a problem hiding this comment.
Will this continue using the name? Or will this switch to using the upstream ID?
There was a problem hiding this comment.
interesting idea, this code goes to repositories.CreateRepository which uses the name as a property for RetrieveAllProperties. I'm not sure if that and especially the underlying code supports retrieving a repo by name. I need to check that.
There was a problem hiding this comment.
It's not a big deal what property is used, as long as we use properties and not tie something to names.
|
I will take the feedback into account, but I'm also going to close this PR and resend it in another PR that will actually also take this change into use. |
Summary
I have a rather large branch that works towards not using database access from
the webhook handler. These two patches were part of that branch and can be submitted separately.
The aim is to decrease the number of attributes we send to the handler as well
as make them more generic.
Related: #4327
Change Type
Testing
as part of a larger branch + unit tests
Review Checklist: