[APP-14739] get org from part for reload #5605
Conversation
viambot
added
the
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
gmulz
changed the title
| return nil, err | ||
| } | ||
|
|
||
| id := ctx.String(generalFlagID) |
There was a problem hiding this comment.
for reload this didn't seem to be initialized ever, so it was always defaulting to "" and so would always get set to the module id. figured we'd just make it explicitly ModuleID in the moduleCloudBuildInfo
There was a problem hiding this comment.
this logic got leftover after a refactor I think, thanks for catching!
stuqdog
left a comment
stuqdog
left a comment
There was a problem hiding this comment.
Generally looks reasonable to me! Though I haven't tested. i assume you did test to ensure that reload still works as expected?
| return nil, err | ||
| } | ||
|
|
||
| id := ctx.String(generalFlagID) |
There was a problem hiding this comment.
this logic got leftover after a refactor I think, thanks for catching!
michaellee1019
left a comment
michaellee1019
left a comment
There was a problem hiding this comment.
The RDK changes LGTM. Lets discuss more of the backend changes, which need to go first, before merging this in.
Also would recommend changing the gRPC API to get the primary org instead of compensating for it on the frontend.
| // module org | ||
| var orgID string | ||
| for _, org := range location.Location.Organizations { | ||
| if org.Primary { | ||
| orgID = org.GetOrganizationId() | ||
| break | ||
| } | ||
| } | ||
| if orgID == "" { | ||
| orgID = location.Location.Organizations[0].GetOrganizationId() | ||
| } |
There was a problem hiding this comment.
Are you sure you don't want to champion for adding primary_org_id to the API? This feels a bit fragile to put on the client side. Its hard to predict the future but if we ever make changes to locations we'd break some range of versions of the CLI.
There was a problem hiding this comment.
agree this way to fetch locations sucks, but was the most workable thing to start with.
are you saying add primary_org_id to the robot collection or actually populate it in the response for GetLocation?
viambot
added
safe to test
updates the reload command to set the organization for the package to be the primary organization of the part, instead of the module's owner org. this allows us to reload modules that do not belong to our own org, if we have the source code for it
APP PR redefining permissions on the cloud (prereq to merge): https://github.com/viamrobotics/app/pull/10599