Duration must be a timedelta instead of string #19427
Conversation
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rakshith91
changed the title
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Outdated
Show resolved
Hide resolved
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
|
||
| def construct_iso8601(start=None, end=None, duration=None): | ||
| try: | ||
| duration = 'PT{}S'.format(duration.total_seconds()) |
There was a problem hiding this comment.
Should we use isinstance?
If user uses a class that has total_seconds property, is it valid?
There was a problem hiding this comment.
What if duration is None in line 69 & 71?
There was a problem hiding this comment.
changed to if duration is not None
- for the second question - it's okay to be None
There was a problem hiding this comment.
Just to confirm, you mean
end = Serializer.serialize_iso(end)
iso_str = None + '/' + endis valid?
There was a problem hiding this comment.
oops - line 69-71 got changed when i made the changes :)
nope = iso_str = None + '/' + end is not a valid format - although end must be provided with a duration
- it's worth improving the message :)
|
|
||
| def query(self, workspace_id, query, duration=None, **kwargs): | ||
| # type: (str, str, str, Any) -> LogsQueryResults | ||
| # type: (str, str, timedelta, Any) -> LogsQueryResults |
There was a problem hiding this comment.
| # type: (str, str, timedelta, Any) -> LogsQueryResults | |
| # type: (str, str, Optional[timedelta], Any) -> LogsQueryResults |
There was a problem hiding this comment.
i'll update all these :)
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-query/azure/monitor/query/_helpers.py
Outdated
Show resolved
Hide resolved
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| metric_names=["PublishSuccessCount"], | ||
| start_time=datetime(2021, 5, 25), | ||
| duration='P1D' | ||
| duration=timedelta(days=1), |
| if end is not None: | ||
| end = Serializer.serialize_iso(end) | ||
| iso_str = start + '/' + end | ||
| elif duration is not None: |
There was a problem hiding this comment.
What's the behavior if user provides start, end and duration?
There was a problem hiding this comment.
we would take the start and end time - we have documented that all three should not be provided - on retrospect, let me warn here too
There was a problem hiding this comment.
In fact my question is if three are provided, we would silently ignore duration (current behavior) or give error? :)
There was a problem hiding this comment.
we now give error :)
|
/azp run python - monitor - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
2 participants
Fixes #19250