-
Notifications
You must be signed in to change notification settings - Fork 5
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
ENH: Add initial new project workflow #302
base: main
Are you sure you want to change the base?
Conversation
Initial workflow for creating an external project in a new domain. Updated the create_project method to include option for domain id and added new domain to user_domains
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #302 +/- ##
==========================================
+ Coverage 98.15% 98.27% +0.11%
==========================================
Files 126 130 +4
Lines 4599 4917 +318
Branches 244 255 +11
==========================================
+ Hits 4514 4832 +318
Misses 76 76
Partials 9 9 ☔ View full report in Codecov by Sentry. |
The |
2677196
to
98e203c
Compare
Pylint was showing errors, formatting the files
98e203c
to
f21c68f
Compare
Add missing unit test for when we have an empty list for admin users Updated jasmin project workflow method to have jasmin users as a mandatory param
Project creation action gets the domain name from the user, add a step in openstack_project to get the id for that given domain to use. The create project method only takes the id for a domain.
lib/structs/project_details.py
Outdated
@dataclass | ||
class ProjectDetails: | ||
name: str | ||
email: str | ||
description: str = "" | ||
domain: str = None |
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.
domain: Optional[str] = None
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.
There's an edit I do in the openstack_project where we need to take a domain id as a required parameter - this is because the openstacksdk doesn't pass the parent id as a domain id so we would need to differentiate between the domains for the projects
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 have a feature branch for the internal project and external project workflow that I'm working on as well that take domain as a parameter (as the domain is a new field in the project creation action)
number_of_floating_ips: int = 1, | ||
number_of_security_group_rules: int = 200, |
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.
both of these need Optional[int]
type hinting
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.
This section isn't part of getting the initial workflow up - to be done in a separate PR (or built on this PR)
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.
TBH I think we should drop these args across the board and hardcode them to whatever ops say rather than passing them as args everywhere
Instead we should make quota changes using the dedicated action later on and start with sensible defaults
It's out of scope for this PR but I'd like to do a follow-on PR that nukes this everywhere
if not admin_user_list: | ||
admin_user_list = [] |
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.
you can write this more concise
admin_user_list = admin_user_list or []
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.
isn't this a direct copy of create_external_project
?
Maybe we could just modify that to accept both generic and jasmin external projects?
The only thing I can see changing is using Jasmin users instead of STFC ones and maybe some more bespoke networking rules
I guess it's fine for now since we're under a deadline - but something to think about
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.
The initial plan was to take a copy of the existing external project workflow and set up the skeleton workflow we need and the implementation of the full workflow will be built on top of it (we need to check the networking workflow section here for this specific project type)
Follows from code suggestion in PR review
In the current draft PR for updating project creation action the variable to set the domain for a project to be created in is project_domain, not domain. Updated the domain variable to be project_domain in workflow
Initial workflow for creating an external project in a new domain. Updated the create_project method to include option for domain id and added new domain to user_domains
Description:
Special Notes:
Submitter:
Have you (where applicable):
Reviewer
Does this PR:
lib
directory?lib
layers?