PR#1417: WIP: Disallow importing modules with virtual streams that overlap - fm-orchestrator

fm-orchestrator

#1417 WIP: Disallow importing modules with virtual streams that overlap

Closed 4 years ago by mprahl. Opened 4 years ago by srieger.

srieger/fm-orchestrator master into master

Disallow importing modules with virtual streams that overlap

sarah256 • 4 years ago

fbe40c8

file modified

		`@@ -1138,6 +1138,11 @@`

		`for stream_name in newly_added_virtual_streams:`
		`virtual_stream = VirtualStream.get_by_name(db_session, stream_name)`
		`+ num_overlapping_streams = \`
		`+ db_session.query(ModuleBuild).filter_by(name=self.name, stream=stream_name).count()`
		`+ if num_overlapping_streams > 0:`
		`+ raise ValueError("Module contains virtual streams that overlap with actual "`
		`+ "streams.")`
		`if not virtual_stream:`
		`virtual_stream = VirtualStream(name=stream_name)`
		`self.virtual_streams.append(virtual_stream)`

tests/test_utils/test_utils.py

file modified

+11 -8

		`@@ -457,14 +457,17 @@`
		`mmd.set_xmd(xmd)`
		`module_build_service.utils.general.import_mmd(db_session, mmd)`

		`- # Import another module which has overlapping virtual streams`
		`- another_mmd = load_mmd(read_staged_data("formatted_testmodule-more-components"))`
		`- # Add some virtual streams to this module metadata`
		`- xmd = another_mmd.get_xmd()`
		`- xmd["mbs"]["virtual_streams"] = ["f29", "f30"]`
		`- another_mmd.set_xmd(xmd)`
		`- another_module_build, _ = module_build_service.utils.general.import_mmd(`
		`- db_session, another_mmd)`
		`+ with pytest.raises(ValueError) as e_info:`
		`+ # Import another module which has overlapping virtual streams`
		`+ another_mmd = load_mmd(read_staged_data("formatted_testmodule-more-components"))`
		`+ # Add some virtual streams to this module metadata`
		`+ xmd = another_mmd.get_xmd()`
		`+ xmd["mbs"]["virtual_streams"] = ["f29", "f30"]`
		`+ another_mmd.set_xmd(xmd)`
		`+ another_module_build, _ = module_build_service.utils.general.import_mmd(`
		`+ db_session, another_mmd)`
		`+ assert e_info.value.args[0] == "Module contains virtual streams that overlap with " \`
		`+ "actual streams."`

		`# Now, remove f30 from mmd`
		`xmd = mmd.get_xmd()`

no initial comment

factory2jenkins commented 4 years ago

Build #405 failed (commit: c382fe93ca2f700c2cbdfac4e89957495f05dcbc).
Rebase or make new commits to rebuild.

mprahl commented on line 5 of module_build_service/models.py 4 years ago

This is invalid syntax. You need a backslash here. The black formatting tool would do the following though:

num_overlapping_streams = (
    db_session.query(cls).filter_by(name=self.name, stream=stream_name).count()
)

mprahl commented on line 8 of module_build_service/models.py 4 years ago

You should only use log.exception within an except handler.

See https://docs.python.org/2/library/logging.html#logging.Logger.exception

mprahl commented on line 9 of module_build_service/models.py 4 years ago

You are just raising the class here. This should be an object of the class instead.

Also, RuntimeError does not accurately describe the issue. How about ValueError?

rebased onto 25e64e1c551fb6c1ca92bb7ac26ea52cddfbf883

4 years ago

factory2jenkins commented 4 years ago

Build #411 failed (commit: 25e64e1c551fb6c1ca92bb7ac26ea52cddfbf883).
Rebase or make new commits to rebuild.

rebased onto e61a98f26a81fb379ef67bed9e118b2cc2686425

4 years ago

factory2jenkins commented 4 years ago

Build #415 failed (commit: e61a98f26a81fb379ef67bed9e118b2cc2686425).
Rebase or make new commits to rebuild.

rebased onto fbe40c8

4 years ago

srieger commented 4 years ago

This is failing on multiple (26) tests, where in each failure it raises the ValueError. I am having a hard time telling whether this was intentional or not to have overlaps in the test data (and if there are overlaps in the test data at all), or whether it is just a fault in my code.

Edited 4 years ago by srieger

factory2jenkins commented 4 years ago

Build #418 failed (commit: fbe40c8).
Rebase or make new commits to rebuild.