Skip to content
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.

Sign up for GitHub

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

Add Metadata to every Maintenance, addresses issue #246 #249

Merged
merged 17 commits into from
Nov 20, 2023
Merged

Add Metadata to every Maintenance, addresses issue #246 #249

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
3aba61f
feat: :sparkles: OpenAI parser
chadell Oct 19, 2023
a4eec2e
refinement in logic
chadell Oct 25, 2023
e7ace76
logic refinement
chadell Oct 25, 2023
c98bf4c
Update README.md
chadell Oct 26, 2023
0f9dcf6
Update circuit_maintenance_parser/parser.py
chadell Oct 26, 2023
5eec491
improve question
chadell Oct 26, 2023
6094f3f
make more explicit the text parsing
chadell Oct 26, 2023
86f8154
Automate token management for local tests
chadell Oct 30, 2023
de559e8
Make openai library an extra
chadell Nov 14, 2023
38c43bf
Adopt OpenAI library changes
chadell Nov 15, 2023
1656bef
fix mypy
chadell Nov 15, 2023
1dbec38
feat: :sparkles: Metadata for every Maintenance
chadell Nov 15, 2023
c328d56
Merge remote-tracking branch 'origin/develop' into issue-246-metadata
chadell Nov 15, 2023
0fe9306
fix tests
chadell Nov 15, 2023
84e5141
update readme
chadell Nov 16, 2023
8510326
Add a boolean in Metadata to reflect LLM parsing
chadell Nov 20, 2023
249ee16
Update circuit_maintenance_parser/parser.py
chadell Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions circuit_maintenance_parser/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,5 @@ def main(provider_type, data_file, data_type, verbose):
for idx, parsed_notification in enumerate(parsed_notifications):
click.secho(f"Circuit Maintenance Notification #{idx}", fg="green", bold=True)
click.secho(parsed_notification.to_json(), fg="yellow")
click.secho(f"Metadata #{idx}", fg="green", bold=True)
click.secho(parsed_notification.metadata, fg="blue")
27 changes: 26 additions & 1 deletion circuit_maintenance_parser/output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from typing import List

from pydantic import BaseModel, validator, StrictStr, StrictInt, Extra
from pydantic import BaseModel, validator, StrictStr, StrictInt, Extra, PrivateAttr


class Impact(str, Enum):
Expand Down Expand Up @@ -91,6 +91,14 @@ def validate_impact_type(cls, value):
return value


class Metadata(BaseModel):
"""Metadata class to provide context about the Maintenance object."""

provider: StrictStr
processor: StrictStr
parsers: StrictStr


class Maintenance(BaseModel, extra=Extra.forbid):
"""Maintenance class.

Expand All @@ -113,6 +121,11 @@ class Maintenance(BaseModel, extra=Extra.forbid):
order

Example:
>>> metadata = Metadata(
... processor="<class 'circuit_maintenance_parser.processor.CombinedProcessor'>",
... provider="genericprovider",
... parsers="[<class 'circuit_maintenance_parser.parser.EmailDateParser'>,]"
... )
>>> Maintenance(
... account="12345000",
... end=1533712380,
Expand All @@ -126,6 +139,7 @@ class Maintenance(BaseModel, extra=Extra.forbid):
... status="COMPLETED",
... summary="This is a maintenance notification",
... uid="1111",
... _metadata=metadata,
... )
Maintenance(provider='A random NSP', account='12345000', maintenance_id='VNOC-1-99999999999', status=<Status.COMPLETED: 'COMPLETED'>, circuits=[CircuitImpact(circuit_id='123', impact=<Impact.NO_IMPACT: 'NO-IMPACT'>), CircuitImpact(circuit_id='456', impact=<Impact.OUTAGE: 'OUTAGE'>)], start=1533704400, end=1533712380, stamp=1533595768, organizer='[email protected]', uid='1111', sequence=1, summary='This is a maintenance notification')
"""
Expand All @@ -139,12 +153,18 @@ class Maintenance(BaseModel, extra=Extra.forbid):
end: StrictInt
stamp: StrictInt
organizer: StrictStr
_metadata: Metadata = PrivateAttr()

# Non mandatory attributes
uid: StrictStr = "0"
sequence: StrictInt = 1
summary: StrictStr = ""

def __init__(self, **data):
"""Initialize the Maintenance object."""
self._metadata = data.pop("_metadata")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This throws KeyError if _metadata key isn't present, thus this change becomes a breaking change.

Some of my existing tests used in my projects start to throw KeyError exception. For example:

self = Maintenance()
data = {'account': '12623', 'circuits': [{'circuit_id': '297487', 'impact': 'NO-IMPACT'}], 'end': 1649073600, 'maintenance_id': 'GIN-CHG0042605', ...}

    def __init__(self, **data):
        """Initialize the Maintenance object."""
>       self._metadata = data.pop("_metadata")
E       KeyError: '_metadata'

../../dev/MNA/lib/python3.10/site-packages/circuit_maintenance_parser/output.py:166: KeyError

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like Maintenance dataclass is now expecting _metadata to be supplied by users. can't it default to None?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @csessh for reporting. I'm going to check and fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checking the code, I see that the _metadata is injected by the Processors automatically, so I don't get why don't you get it in your tests.

super().__init__(**data)

# pylint: disable=no-self-argument
@validator("status")
def validate_status_type(cls, value):
Expand Down Expand Up @@ -185,3 +205,8 @@ def slug(self) -> str:
def to_json(self) -> str:
"""Get JSON representation of the class object."""
return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=2)

@property
def metadata(self):
"""Get Maintenance Metadata."""
return self._metadata
10 changes: 9 additions & 1 deletion circuit_maintenance_parser/processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from pydantic import BaseModel, Extra
from pydantic.error_wrappers import ValidationError

from circuit_maintenance_parser.output import Maintenance
from circuit_maintenance_parser.output import Maintenance, Metadata
from circuit_maintenance_parser.data import NotificationData
from circuit_maintenance_parser.parser import Parser
from circuit_maintenance_parser.errors import ParserError, ProcessorError
Expand Down Expand Up @@ -98,6 +98,12 @@ def extend_processor_data(self, current_maintenance_data):
current_maintenance_data.update(self.extended_data)
current_maintenance_data.update(temp_res)

def generate_metadata(self):
"""Generate the Metadata for the Maintenance."""
return Metadata(
parsers=str(self.data_parsers), processor=str(self.__class__), provider=self.extended_data["provider"]
)


class SimpleProcessor(GenericProcessor):
"""Processor to get all the Maintenance Data in each Data Part."""
Expand All @@ -106,6 +112,7 @@ def process_hook(self, maintenances_extracted_data, maintenances_data):
"""For each data extracted (that can be multiple), we try to build a complete Maintenance."""
for extracted_data in maintenances_extracted_data:
self.extend_processor_data(extracted_data)
extracted_data["_metadata"] = self.generate_metadata()
maintenances_data.append(Maintenance(**extracted_data))


Expand Down Expand Up @@ -143,6 +150,7 @@ def post_process_hook(self, maintenances_data):
for maintenance in maintenances:
try:
combined_data = {**self.combined_maintenance_data, **maintenance}
combined_data["_metadata"] = self.generate_metadata()
maintenances_data.append(Maintenance(**combined_data))
except ValidationError as exc:
raise ProcessorError("Not enough information available to create a Maintenance notification.") from exc
2 changes: 2 additions & 0 deletions tests/unit/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Used to setup fixtures to be used through tests"""
import pytest
from circuit_maintenance_parser.output import Metadata


@pytest.fixture()
Expand All @@ -19,6 +20,7 @@ def maintenance_data():
"status": "COMPLETED",
"summary": "This is a maintenance notification",
"uid": "VNOC-1-99999999999",
"_metadata": Metadata(provider="some provider", processor="some processor", parsers="some parsers"),
}


Expand Down
76 changes: 70 additions & 6 deletions tests/unit/test_processors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import pytest
from pydantic.error_wrappers import ValidationError

from circuit_maintenance_parser.output import Maintenance
from circuit_maintenance_parser.output import Maintenance, Metadata
from circuit_maintenance_parser.processor import CombinedProcessor, SimpleProcessor
from circuit_maintenance_parser.data import DataPart, NotificationData
from circuit_maintenance_parser.errors import ProcessorError
Expand All @@ -16,7 +16,7 @@
# pylint: disable=global-variable-undefined

PARSED_DATA = [{"a": "b"}, {"c": "d"}]
EXTENDED_DATA = {"y": "z"}
EXTENDED_DATA = {"y": "z", "provider": "required"}


class FakeParser(Parser):
Expand Down Expand Up @@ -72,6 +72,13 @@ def test_simpleprocessor():
assert mock_maintenance.call_count == len(PARSED_DATA)
for parsed_data_element in PARSED_DATA:
parsed_data_element.update(EXTENDED_DATA)
parsed_data_element.update(
{
"_metadata": Metadata(
provider=EXTENDED_DATA["provider"], processor=str(SimpleProcessor), parsers=str([FakeParser])
)
}
)
mock_maintenance.assert_any_call(**parsed_data_element)


Expand All @@ -92,6 +99,13 @@ def test_combinedprocessor_multiple_data():
assert mock_maintenance.call_count == len(PARSED_DATA)
for parsed_data_element in PARSED_DATA:
parsed_data_element.update(EXTENDED_DATA)
parsed_data_element.update(
{
"_metadata": Metadata(
provider=EXTENDED_DATA["provider"], processor=str(CombinedProcessor), parsers=str([FakeParser])
)
}
)
mock_maintenance.assert_any_call(**parsed_data_element)


Expand All @@ -102,7 +116,20 @@ def test_combinedprocessor():
with patch("circuit_maintenance_parser.processor.Maintenance") as mock_maintenance:
processor.process(fake_data_for_combined, EXTENDED_DATA)
assert mock_maintenance.call_count == 1
mock_maintenance.assert_any_call(**{**PARSED_DATA[0], **PARSED_DATA[1], **EXTENDED_DATA})
mock_maintenance.assert_any_call(
**{
**PARSED_DATA[0],
**PARSED_DATA[1],
**EXTENDED_DATA,
**{
"_metadata": Metadata(
provider=EXTENDED_DATA["provider"],
processor=str(CombinedProcessor),
parsers=str([FakeParser0, FakeParser1]),
)
},
}
)


def test_combinedprocessor_missing_data():
Expand All @@ -125,12 +152,37 @@ def test_combinedprocessor_bleed():
with patch("circuit_maintenance_parser.processor.Maintenance") as mock_maintenance:
processor.process(fake_data_for_combined, EXTENDED_DATA)
assert mock_maintenance.call_count == 1
mock_maintenance.assert_called_with(**{**PARSED_DATA[0], **PARSED_DATA[1], **EXTENDED_DATA})
mock_maintenance.assert_any_call(
**{
**PARSED_DATA[0],
**PARSED_DATA[1],
**EXTENDED_DATA,
**{
"_metadata": Metadata(
provider=EXTENDED_DATA["provider"],
processor=str(CombinedProcessor),
parsers=str([FakeParser0, FakeParser1]),
)
},
}
)

with patch("circuit_maintenance_parser.processor.Maintenance") as mock_maintenance:
processor.process(fake_data_type_0, EXTENDED_DATA)
assert mock_maintenance.call_count == 1
mock_maintenance.assert_called_with(**{**PARSED_DATA[0], **EXTENDED_DATA})
mock_maintenance.assert_called_with(
**{
**PARSED_DATA[0],
**EXTENDED_DATA,
**{
"_metadata": Metadata(
provider=EXTENDED_DATA["provider"],
processor=str(CombinedProcessor),
parsers=str([FakeParser0, FakeParser1]),
)
},
}
)


def test_combinedprocessor_multidatatype():
Expand All @@ -141,5 +193,17 @@ def test_combinedprocessor_multidatatype():
with patch("circuit_maintenance_parser.processor.Maintenance") as mock_maintenance:
processor.process(fake_data_for_combined, EXTENDED_DATA)
assert mock_maintenance.call_count == 1
mock_maintenance.assert_any_call(**{**PARSED_DATA[1], **EXTENDED_DATA})
mock_maintenance.assert_any_call(
**{
**PARSED_DATA[1],
**EXTENDED_DATA,
**{
"_metadata": Metadata(
provider=EXTENDED_DATA["provider"],
processor=str(CombinedProcessor),
parsers=str([FakeParserMultiDataType]),
)
},
}
)
assert parser_runs == 1