PR#1347: WIP: reuse for Valerij - fm-orchestrator

fm-orchestrator

#1347 WIP: reuse for Valerij

Closed 4 years ago by jkaluza. Opened 4 years ago by jkaluza.

jkaluza/fm-orchestrator reuse into master

WIP: reuse for Valerij

Jan Kaluza • 4 years ago

508bf67

module_build_service/utils/reuse.py

file modified

+39 -18

		`@@ -26,6 +26,8 @@`

		`import module_build_service.messaging`
		`from module_build_service import log, models, conf`
		`+ from module_build_service.utils.general import mmd_to_str`
		`+ from module_build_service.utils.mse import _get_base_module_mmds`


		`def reuse_component(component, previous_component_build, change_state_now=False):`
		`@@ -86,28 +88,47 @@`
		`:return: ModuleBuild object which can be used for component reuse.`
		`"""`

		`+ log.info('get reusable module called')`
mprahl commented 4 years ago Could you remove this?
		`if module.reused_module:`
		`return module.reused_module`

		`mmd = module.mmd()`
		`- # Find the latest module that is in the done or ready state`
		`- previous_module_build = (`
		`- session.query(models.ModuleBuild)`
		`- .filter_by(name=mmd.get_module_name())`
		`- .filter_by(stream=mmd.get_stream_name())`
		`- .filter_by(state=models.BUILD_STATES["ready"])`
		`- .filter(models.ModuleBuild.scmurl.isnot(None))`
		`- .filter_by(build_context=module.build_context)`
		`- .order_by(models.ModuleBuild.time_completed.desc())`
		`- )`
		`- # If we are rebuilding with the "changed-and-after" option, then we can't reuse`
		`- # components from modules that were built more liberally`
		`- if module.rebuild_strategy == "changed-and-after":`
		`- previous_module_build = previous_module_build.filter(`
		`- models.ModuleBuild.rebuild_strategy.in_(["all", "changed-and-after"]))`
		`- previous_module_build = previous_module_build.filter_by(`
		`- ref_build_context=module.ref_build_context)`
		`- previous_module_build = previous_module_build.first()`
		`+ previous_module_build = None`
		`+`
		`+ base_mmds = _get_base_module_mmds(mmd)["ready"]`
		`+ for base_mmd in base_mmds:`
		`+ if base_mmd.get_module_name() not in mmd.get_xmd()["mbs"]["buildrequires"]:`
		`+ continue`
		`+ print "original context:", module.build_context`
		`+ mmd.get_xmd()["mbs"]["buildrequires"][base_mmd.get_module_name()]["stream"] \`
mprahl commented 4 years ago This line won't change the xmd value. You'd need to do the following: xmd = mmd.get_xmd() xmd["mbs"]["buildrequires"][base_mmd.get_module_name()]["stream"] = base_mmd.get_stream_name() mmd.set_xmd(xmd) With that said, you should not directly modify that mmd file. If you continue with this approach, you should created a copy. For example: temp_mmd = mmd.copy()
mprahl commented 4 years ago Instead of creating all these temporary Modulemd objects, it might be cleaner to add an overrides keyword argument to `contexts_from_mmd`. This argument would take a dictionary and update the `mmd.get_xmd()["mbs"]["buildrequires"]` dictionary with the overrides.
		`+ = base_mmd.get_stream_name()`
		`+ print('contexts: {}'.format(module.contexts_from_mmd(mmd_to_str(mmd))))`
		`+ print('module_build_context: {}'.format(module.build_context))`
		`+ build_context = module.contexts_from_mmd(mmd_to_str(mmd))[1]`
		`+ # Find the latest module that is in the done or ready state`
		`+ previous_module_build = (`
		`+ session.query(models.ModuleBuild)`
		`+ .filter_by(name=mmd.get_module_name())`
		`+ .filter_by(stream=mmd.get_stream_name())`
		`+ .filter_by(state=models.BUILD_STATES["ready"])`
		`+ .filter(models.ModuleBuild.scmurl.isnot(None))`
		`+ .filter_by(build_context=build_context)`
		`+ .order_by(models.ModuleBuild.time_completed.desc()))`
		`+`
		`+ # If we are rebuilding with the "changed-and-after" option, then we can't reuse`
		`+ # components from modules that were built more liberally`
		`+ if module.rebuild_strategy == "changed-and-after":`
		`+ previous_module_build = previous_module_build.filter(`
		`+ models.ModuleBuild.rebuild_strategy.in_(["all", "changed-and-after"])`
		`+ )`
		`+ previous_module_build = previous_module_build.filter_by(`
		`+ ref_build_context=module.ref_build_context`
		`+ )`
		`+ previous_module_build = previous_module_build.first()`
		`+`
		`+ if previous_module_build:`
		`+ break`
		`+`
		`# The component can't be reused if there isn't a previous build in the done`
		`# or ready state`
		`if not previous_module_build:`

tests/__init__.py

file modified

+4 -2

		`@@ -452,7 +452,6 @@`
		`state=BUILD_STATES["ready"],`
		`ref_build_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0",`
		`runtime_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0",`
		`- build_context="ac4de1c346dcf09ce77d38cd4e75094ec1c08eb1",`
		`context="78e4a6fd",`
		`koji_tag="module-testmodule-master-20170109091357-78e4a6fd",`
		`scmurl="https://src.stg.fedoraproject.org/modules/testmodule.git?#ff1ea79",`
		`@@ -472,6 +471,8 @@`
		`xmd["mbs"]["commit"] = "ff1ea79fc952143efeed1851aa0aa006559239ba"`
		`mmd.set_xmd(xmd)`
		`build_one.modulemd = mmd_to_str(mmd)`
		`+ build_one.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd(`
		`+ build_one.modulemd)[1]`

		`db.session.add(build_one)`
		`db.session.commit()`
		`@@ -572,6 +573,8 @@`
		`xmd["mbs"]["commit"] = "55f4a0a2e6cc255c88712a905157ab39315b8fd8"`
		`mmd.set_xmd(xmd)`
		`build_two.modulemd = mmd_to_str(mmd)`
		`+ build_two.build_context = module_build_service.models.ModuleBuild.contexts_from_mmd(`
		`+ build_one.modulemd)[1]`

		`db.session.add(build_two)`
		`db.session.commit()`
		`@@ -641,7 +644,6 @@`
		`name=mmd.get_module_name(),`
		`stream=mmd.get_stream_name(),`
		`version=mmd.get_version(),`
		`- build_context="e046b867a400a06a3571f3c71142d497895fefbe",`
		`runtime_context="50dd3eb5dde600d072e45d4120e1548ce66bc94a",`
		`state=BUILD_STATES["ready"],`
		`modulemd=mmd_to_str(mmd),`

tests/test_utils/test_utils.py

file modified

+6 -4

		`@@ -604,8 +604,10 @@`
		`reuse the components.`
		`"""`
		`reuse_shared_userspace_init_data()`
		`+ old_module = models.ModuleBuild.get_by_id(db_session, 2)`
		`new_module = models.ModuleBuild.get_by_id(db_session, 3)`
		`- rv = module_build_service.utils.get_reusable_component(db.session, new_module, "llvm")`
		`+ rv = module_build_service.utils.get_reusable_component(`
		`+ db.session, new_module, "llvm", previous_module_build=old_module)`
		`assert rv.package == "llvm"`

		`def test_validate_koji_tag_wrong_tag_arg_during_programming(self):`
		`@@ -1431,14 +1433,14 @@`
		`class TestUtilsModuleReuse:`

		`def setup_method(self, test_method):`
		`- init_data()`
		`+ reuse_component_init_data()`

		`def teardown_method(self, test_method):`
		`clean_database()`

		`def test_get_reusable_module_when_reused_module_not_set(self):`
		`module = models.ModuleBuild.query.filter_by(`
		`- name="nginx").order_by(models.ModuleBuild.id.desc()).first()`
		`+ name="testmodule").order_by(models.ModuleBuild.id.desc()).first()`
		`module.state = models.BUILD_STATES["build"]`
		`db.session.commit()`

		`@@ -1452,7 +1454,7 @@`

		`def test_get_reusable_module_when_reused_module_already_set(self):`
		`modules = models.ModuleBuild.query.filter_by(`
		`- name="nginx").order_by(models.ModuleBuild.id.desc()).limit(2).all()`
		`+ name="testmodule").order_by(models.ModuleBuild.id.desc()).limit(2).all()`
		`build_module = modules[0]`
		`reused_module = modules[1]`
		`build_module.state = models.BUILD_STATES["build"]`

no initial comment

mprahl commented 4 years ago

@vmaljulin could you remove the print statements please?

mprahl commented on line 14 of module_build_service/utils/reuse.py 4 years ago

Could you remove this?

Pull-Request has been closed by jkaluza

4 years ago

mprahl commented on line 44 of module_build_service/utils/reuse.py 4 years ago

This line won't change the xmd value. You'd need to do the following:

xmd = mmd.get_xmd()
xmd["mbs"]["buildrequires"][base_mmd.get_module_name()]["stream"] = base_mmd.get_stream_name()
mmd.set_xmd(xmd)

With that said, you should not directly modify that mmd file. If you continue with this approach, you should created a copy. For example:

temp_mmd = mmd.copy()

mprahl commented on line 44 of module_build_service/utils/reuse.py 4 years ago

Instead of creating all these temporary Modulemd objects, it might be cleaner to add an overrides keyword argument to contexts_from_mmd. This argument would take a dictionary and update the mmd.get_xmd()["mbs"]["buildrequires"] dictionary with the overrides.