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.

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

Update to use timespan #20233

Merged
merged 7 commits into from
Aug 16, 2021
Merged

Update to use timespan #20233

merged 7 commits into from
Aug 16, 2021

Conversation

rakshith91
Copy link
Contributor

No description provided.

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

elif isinstance(timespan[1], timedelta):
start, duration = timespan[0], timespan[1]
else:
raise ValueError('Tuple must be a start datetime with a timedelta or an end datetime.')
Copy link
Member

Choose a reason for hiding this comment

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

Could you add some comments to describe the situation?
e.g. if isinstance(timespan[1], datetime): # if timespan is tuple[datetime, datetime], we treat it as [start time, end time].

else:
raise ValueError('Tuple must be a start datetime with a timedelta or an end datetime.')
except TypeError:
duration = timespan
if duration is not None:
Copy link
Member

Choose a reason for hiding this comment

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

We can just use if duration: :)

iso_str = None
if start is not None:
start = Serializer.serialize_iso(start)
if end and duration:
raise ValueError("start_time can only be provided with duration or end_time, but not both.")
if end is not None:
end = Serializer.serialize_iso(end)
iso_str = start + '/' + end
elif duration is not None:
iso_str = start + '/' + duration
else:
Copy link
Member

Choose a reason for hiding this comment

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

In which case will enter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

premeptively raising in case start is provided with something other than a timedelta or a datetime

Copy link
Member

Choose a reason for hiding this comment

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

It looks to me here is the case start is not None while end and duration are None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

improved the message to warn about none explicitly

if not duration:
raise ValueError("End time must be provided along with duration or start time.")
end = Serializer.serialize_iso(end)
iso_str = duration + '/' + end
else:
Copy link
Member

Choose a reason for hiding this comment

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

Also, in which case will enter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in case timespan is a timedelta and no start is provided

@rakshith91
Copy link
Contributor Author

/azp run python - monitor - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakshith91 rakshith91 merged commit d5ce554 into Azure:main Aug 16, 2021
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Aug 23, 2021
…into have_pipelines_support_rest

* 'main' of https://github.com/Azure/azure-sdk-for-python: (108 commits)
  Enable API review approval check for Java spring packages (Azure#20311)
  [AutoRelease] t2-iothubprovisioningservices-2021-07-15-81882 (Azure#19816)
  Increment package version after release of azure-eventgrid (Azure#20204)
  Sync eng/common directory with azure-sdk-tools for PR 1909 (Azure#20298)
  Rename attrs is metrics (Azure#20236)
  [EventHubs] checkpointstoretable - skip tests until env vars configured (Azure#20289)
  Update to use timespan (Azure#20233)
  Prevent wildcard expansion in git sparse checkout add (Azure#20267)
  Fix typo and polish the key concepts (Azure#18407)
  Fix IOT Device Update readme issue (Azure#18752)
  Add context manager API to azure.identity credentials (Azure#19746)
  Add support for 'files' configuration (Azure#20272)
  Update main for 30-close.py (Azure#20287)
  [AutoRelease] t2-purview-2021-08-13-30358 (Azure#20260)
  add implementation for checkpointstoretable (Azure#19905)
  Fix resource clean-up script (Azure#20273)
  [Tables] Fix bug in update mode (Azure#20264)
  Add Rest Method checks to Prepare-Release (Azure#20275)
  Update release date (Azure#20270)
  Update the release date for ACS chat 1.1.0b1 (Azure#20277)
  ...
iscai-msft pushed a commit that referenced this pull request Sep 29, 2021
* Update to use timespan

* lint

* lint

* lint + test

* Update test_logs_client.py

* comments

* improve message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants