Conversation
|
| # pip-compile --upgrade --no-emit-index-url | ||
| # Install using: | ||
| # pip-sync |
There was a problem hiding this comment.
I'm all up for using uv and dropping pip-tools as you've done now in this PR. But I also think that the team should standardize on one tool since they might resolve dependencies slightly differently.
So probably would be beneficial to change these to:
# uv pip compile requirements.in -o requirements.txt --upgrade --no-emit-index-url
# Install using:
# uv pip sync requirements.txt
And the same for requirements-dev.in
| @@ -103,7 +103,9 @@ async def publish_messages(self, messages: list, publishWIS2: bool, baseURL: str | |||
| if publishWIS2 and self.WIS2_client: | |||
| send_message( | |||
| generate_wis2_topic(), | |||
There was a problem hiding this comment.
This is not part of the PR, but as the topic is very much static, maybe it should not be called every time a message is published or at least minimize the overhead if we want to keep the ValueError raising?
What if we change the generate_wis2_topic() to something like:
_WIS2_TOPIC = None
def generate_wis2_topic() -> str:
"""This function will generate the WIS2 compliant topic name"""
global _WIS2_TOPIC
if _WIS2_TOPIC is None:
_WIS2_TOPIC = os.getenv("WIS2_TOPIC")
if not _WIS2_TOPIC:
raise ValueError("WIS2_TOPIC env variable not set. Aborting publish to wis2")
return _WIS2_TOPIC
"Cached" version should be significantly faster because it does not make a os.getenv() dictionary lookup every time and as the messages will be in the millions per day, it matters.
| integrity: Optional[Integrity] = Field(None, exclude_from_schema=True) | ||
|
|
||
| @model_validator(mode="after") | ||
| def set_dateimte(self): |
There was a problem hiding this comment.
| def set_dateimte(self): | |
| def set_datetime(self): |
| data_id=os.getenv("WIS2_DATA_ID") or message["properties"]["data_id"], | ||
| metadata_id=os.getenv( | ||
| "WIS2_METADATA_RECORD_ID", None |
There was a problem hiding this comment.
These env variables could also be cached
| ) | ||
| content: Optional[Content] = Field(..., description="Actual data content") | ||
| metadata_id: Optional[str] = Field( | ||
| ..., |
There was a problem hiding this comment.
This should be metadata_id: Optional[str] = Field(None, description="..." for it to be optional, right?
Field(..., ...) makes it required.
| @model_validator(mode="after") | ||
| def set_dateimte(self): | ||
| assert bool(self.datetime) != bool(self.start_datetime and self.end_datetime), ( | ||
| "Set datetime or start_datetime and end_datetime. At least one and not both. " |
There was a problem hiding this comment.
Could this be a bit clearer that it needs to be either datetime OR start_datetime and end_datetime. Maybe its just me, but the "At least one and not both" to be sounds like it could also be just the end_time which contradicts the assert.
So what about:
"Set either 'datetime' OR both 'start_datetime' and 'end_datetime', not both options."
| ## Dev install | ||
|
|
||
| To install in dev mode run `pip install --editable .` from the top level of this repository. |
There was a problem hiding this comment.
Related to the one tool standardization.
| ## Dev install | |
| This project uses **uv** for dependency management. | |
| To install in dev mode run `uv pip install -e .` from the top level of this repository. |
|
|
||
| class Integrity(BaseModel): | ||
| method: Literal["sha256", "sha384", "sha512", "sha3-256", "sha3-384", "sha3-512"] | ||
| value: str = Field(..., desciption="The hash value for the value field in the message content.") |
There was a problem hiding this comment.
Noticed a typo here also.
| value: str = Field(..., description="The hash value for the value field in the message content.") |
2 participants
Some updates to conform to wis2 message formatting.
Also makes it possible to set the data_id for wis2.