-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update protocols to match latest core schema #107
Conversation
@@ -397,7 +399,6 @@ class CoreNumberPool(CoreResourcePool, LineageSource): | |||
|
|||
|
|||
class CoreObjectPermission(CoreBasePermission): | |||
branch: String |
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.
@gmazoyer is this change expected ? it wasn't part of my change
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.
That should be fine as we decided to remove the "branch" parts as part of the object permissions with the argument that users probably wouldn't want to setup permissions on specific branches and instead have settings for "allow on default branch" or "allow on other branches" on in the future "allow on branches that I have created".
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #107 +/- ##
========================================
Coverage 64.27% 64.27%
========================================
Files 74 74
Lines 6856 6856
Branches 1355 1355
========================================
Hits 4407 4407
Misses 2098 2098
Partials 351 351
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -7,7 +7,8 @@ | |||
from .protocols_base import CoreNode, CoreNodeSync | |||
|
|||
if TYPE_CHECKING: | |||
from .node import RelatedNode, RelatedNodeSync, RelationshipManager, RelationshipManagerSync | |||
from infrahub_sdk.node import RelatedNode, RelatedNodeSync, RelationshipManager, RelationshipManagerSync |
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.
Was there some problem with the relative import here?
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 haven't touched this part, not sure why the import was relative in the first place
this is what the current code is generating
Related to opsmill/infrahub#4716