-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
Introduce Job approach and JobDependency #183
base: main
Are you sure you want to change the base?
Conversation
0f08475
to
def4697
Compare
9a876e0
to
eb4032a
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #183 +/- ##
==========================================
+ Coverage 78.21% 78.74% +0.52%
==========================================
Files 47 47
Lines 5413 5388 -25
==========================================
+ Hits 4234 4243 +9
+ Misses 1179 1145 -34 ☔ View full report in Codecov by Sentry. |
eb4032a
to
f8656e5
Compare
Also apply few reformat
Inside the CI we apply filter for pytest and some were ignored.
for distribution in distributions: | ||
for component in components: | ||
for plugin in plugins: | ||
if "DistributionComponentPlugin" in [ |
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.
Using isinstance()
here would lead to cyclic imports, right?
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 that was the issue, I can double check.
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 just remind that the problem is because plugins inherit from multiple classes and it looks like isinstance
does not find parent class type.
qubesbuilder/pluginmanager.py
Outdated
@@ -75,7 +75,7 @@ def _get_plugin_entities(self): | |||
|
|||
return entities | |||
|
|||
def _get_instances_with_attr(self, module_attr, **kwargs): | |||
def _get_instances_with_attr(self, module_attr): |
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 no longer return instances, but plugin classes.
qubesbuilder/plugins/__init__.py
Outdated
if dependency.builder_object == "component": | ||
if dependency.builder_object == "plugin": | ||
if ( | ||
dependency.builder_object == "plugin" |
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.
redundant
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.
yes copy paste..
qubesbuilder/plugins/__init__.py
Outdated
) | ||
if not component: | ||
raise PluginError( | ||
f"Cannot find component '{dependency}' in configuration file." | ||
) | ||
if not self.config.sources_dir / dependency.name: | ||
if not self.config.sources_dir / dependency.reference: |
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.
missing .exists()
?
""" | ||
Run plugin for given stage. | ||
""" | ||
|
||
if stage != "fetch": | ||
if self.stage != "fetch": |
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.
Is this still necessary with stages
defined at the class level?
|
||
if stage != "sign" or not self.has_component_packages("sign"): | ||
if self.stage != "sign" or not self.has_component_packages("sign"): |
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.
and here
|
||
if stage != "prep" or not self.has_component_packages("prep"): | ||
if self.stage != "prep" or not self.has_component_packages("prep"): |
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.
and here
|
||
if stage != "prep" or not self.has_component_packages("prep"): | ||
if self.stage != "prep" or not self.has_component_packages("prep"): |
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.
and here
|
||
if stage != "prep" or not self.has_component_packages("prep"): | ||
if self.stage != "prep" or not self.has_component_packages("prep"): |
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.
and here
def run(self, stage: str, repository_publish: str = None): | ||
if stage != "upload": | ||
def run(self, repository_publish: Optional[str] = None): | ||
if self.stage != "upload": |
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.
and here
Looks good now I think. Can you schedule a pipeline with QubesOS/qubes-builderv2-github#23 on this PR? |
No description provided.