-
Notifications
You must be signed in to change notification settings - Fork 40
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
Remove domain model (part 1 CLI) #1137
Conversation
One of two PRs to remove the domain model before we launch As discussed on slack, this PR strips out client side cmds for all CRUD type operations in medic around orgs, roles, users , projects. I also removed the old key generation code which we are not using at present and have no immediate need to use. Following this will be a PR to remove / disable the equivalent APIs Signed-off-by: Luke Hinds <luke@stacklok.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, the rest looks good
{"Organization ID", newUser.OrganizationId}, | ||
{"Organization Name", newUser.OrganizatioName}, | ||
{"Project ID", newUser.ProjectId}, | ||
{"Project Name", newUser.ProjectName}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to remove the project information? Folks will not be able to set it but it would be handy to get folks used to this information, for when we have a full SaaS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not of to strong an opinion, I figured people might raise questions 'what can I do with a project', but I guess that is a good opportunity to tell them are plans for a domain model. I will include them again.
Signed-off-by: Luke Hinds <luke@stacklok.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go, left one question about apply, but we can always decide to nuke that one too in a later PR.
} else if object.Object == "project" { | ||
project.Project_createCmd.Run(cmd, args) | ||
} else { | ||
fmt.Fprintf(os.Stderr, "Error: unknown object type %s\n", object.Object) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is apply good for now? Do we anticipate adding support e.g. for profile apply?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to cover later, I have tracked your comment for now.
@lukehinds feel free to merge this |
One of two PRs to remove the domain model before we launch
As discussed on slack, this PR strips out client side cmds for all CRUD type operations in medic around orgs, roles, users , projects.
I also removed the old key generation code which we are not using at present and have no immediate need to use.
Following this will be a PR to remove / disable the equivalent APIs
Closes: #1140