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

Jump to bottom

clean up temp file after run #9356

Merged
merged 2 commits into from
Jan 8, 2020
Merged

clean up temp file after run #9356

merged 2 commits into from
Jan 8, 2020

Conversation

xiangyan99
Copy link
Member

@xiangyan99 xiangyan99 requested a review from bryevdv as a code owner January 7, 2020 23:21
bryevdv
bryevdv reviewed Jan 7, 2020
View reviewed changes
sdk/core/azure-core/tests/test_retry_policy.py Outdated
http_retry = RetryPolicy(retry_total=1)
pipeline = Pipeline(MockTransport(), [http_retry])
pipeline.run(http_request)
os.remove(file_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would with tempfile.NamedTemporaryFile work here? If it does, it would clean up after itself

Copy link
Member Author

Choose a reason for hiding this comment

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

If I understand correctly, tempfile will be removed automatically after close. Hence it will be gone after
with open(file_name, "w+") as f:
f.write('Lots of dataaaa')?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but couldn't that write occur in the tempfile with block as the first action?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good question. And in fact I purposely separate them. :)

My thought is

file_name = str(uuid.uuid4())
with open(file_name, "w+") as f:
f.write('Lots of dataaaa')

are the test preparation steps and after that are real test scenario. I don't want to mix them though I don't think it will impact the results.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wish the docs were more clear about this. They state:

Whether the name can be used to open the file a second time, while the named temporary file is still open, varies across platforms

Which actually leads me to believe that using NamedTemporaryFile to create a file which you write data to and then close, would be OK to open again later. That might be preferable to saving a file with a UUID filename in the current dir. There are definitely cases where writing the file might fail (non-writable curdir) or deleting may fail (an exception in the test) leaving junk in the repo (which always risks being checked in by mistake).

Which I guess raises another avenue: why not just check in a sample data file in to the test directory?

Copy link
Member Author

Choose a reason for hiding this comment

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

tempfile has a delete parameter which is exactly what we want. update the case to use tempfile

@xiangyan99 xiangyan99 requested a review from bryevdv January 8, 2020 17:10
@bryevdv
Copy link
Contributor

bryevdv commented Jan 8, 2020

Maybe bump CI in case the Cosmos emulator test fail was spurious? Otherwise we should let those folks know that it's out of sync again

@xiangyan99
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

bryevdv
bryevdv previously approved these changes Jan 8, 2020
View reviewed changes
@xiangyan99 xiangyan99 requested review from lmazuel and bryevdv January 8, 2020 21:13
@xiangyan99
Copy link
Member Author

/azp run python - cosmos - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@xiangyan99 xiangyan99 merged commit 98993ce into master Jan 8, 2020
@xiangyan99 xiangyan99 deleted the core_cleanup_tempfile branch January 8, 2020 23:09
xiangyan99 added a commit that referenced this pull request Jan 14, 2020
@xiangyan99 @scbedd
@Petermarcu @yunhaoling @kristapratico @chlowell @iscai-msft @AutorestCI @openapi-sdkautomation @lmazuel @weshaggard @YijunXieMS @azuresdkci
Get latest master (#9441)
1426289 
* Index Creation Bugfixes (#9334)

* porting updates for azure-mgmt-web namespaces. updating index creation script to include replace relative links. small updates to package json. updating documentation landing page with appropriate link text and location.
* updating 404ing reference to cspack tool, resolving 404ing link to powershell cmdlet, removing pymodule index from index, remove links to some standard sphinx constructs that won't stick around anymore, override reference to 404ing roboto-slab font.

* Run Tox Test Environments in Parallel (#9034)

* take advantage of improvements to tox-monorepo, plumb through tox environments in parallel
* update tox.ini to only use `envtmpdir` instead of `distdir` for temp work.
* disabling coverage in sdist environment
* cogservices textanalytics uses core
* changing the whl to use envtmpdir
* replacing dev_requirements during CI build. this should  mitigate CI failures due to parallelization of install during tox runs
* ignoring the .tox directory to stop pytest discovery errors during collection

* Update issue templates (#9353)

Add issue and feature templates, mirroring the pattern used in Azure JS SDKs.

* [EventHubs] Sample, Doc, Test improvement (#9349)

* revert back and improve comments

* remove livetest mark for tests which are not livetest

* update docstring and readme

* readme and sample improvement

* update migration guide

* minor update on history

* review update

* small fix of samples

* [textanalytics] update history.md (#9368)

* update history for TA preview release

* add note to track1

* formatting

* add contributing for readme (#9369)

* Remove enter and exit from async context managers (#9313)

* Adding testutils to defined packages within azure-common (#9365)

* adding testutils to defined packages within azure-common
* moving testutils out of azure-common and into azure-sdk-tools

* updated version and changelog (#9373)

* [textanalytics] Add azure-ai-nspkg to artifacts in ci.yml (#9371)

* update ci.yml to include nspkg

* newline

* add setup.cfg to nspkg

* readme tweaks and fix innererror code returned

* clean up temp file after run (#9356)

* clean up temp file after run

* use tempfile

* Fix certs readme (#9370)

* Improve DefaultAzureCredential docstring and error message (#9376)

* Categorize Text Analytics (#9377)

* categorize textanalytics

* Manually generate iothub (#9378)

* multi-api package

* history and version

* Packaging update of azure-mgmt-iothub

* rerecorded tests

Co-authored-by: Azure SDK Bot <[email protected]>

* Restapi auto databoxedge/resource manager (#9367)

* [AutoPR databoxedge/resource-manager] chore: jsonfmt databoxedge (#7361)

* Generated from 8b79ae248e5093f824f1c3c0df81a20a3b3cd4de

chore: jsonfmt databoxedge

* Packaging update of azure-mgmt-databoxedge

* [AutoPR databoxedge/resource-manager]   Add new version of API with object store, sku selection and arm lite user (#8735)

* Generated from b9b42fd9797234b52ae3f05f93711067560d2544

Fixed comments on Sku API

* Generated from b502551c6a94008e85be6c3a666f7f3edfc2b67f

Removed unused property

* Generated from b288943f733062a70176e972eeb92e46c85fa6a1

"Changed operationId for skus API"

* databoxedge generated multi-api

* history and version

* Packaging update of azure-mgmt-databoxedge

* added pipeline

Co-authored-by: Azure SDK Bot <[email protected]>

* [EventHubs] Comment, Test Updates  (#9366)

* update docstring

* await task to prevent potential seg fault

* review update

* PM review update

* review update

* review update

* minor improvement

* minor improvement

* Generated from 1998d0a6549d02cceed76351dbbbfc131382fa80 (#9382)

Adding all files, examples (removed the readonly from encryption services for table and queue)

* [AutoPR storagesync/resource-manager] Microsoft.StorageSync 2019-06-01 API version (#7846)

* Generated from a2749eec38ba462433316a22161df798373732f2

Updates API version in new specs and examples

* Generated from 565b2bebd47ef8fc6b4eba421fe5cc863d15e062

Update StorageSync 2019-06-01 protocol with cloud tiering and recall status (#7161)

* introduce cloud tiering and recall status

* fix validation errors

* Additional validation fixes

* Packaging update of azure-mgmt-storagesync

* Packaging update of azure-mgmt-storagesync

* history and version

Co-authored-by: Zim Kalinowski <[email protected]>

* history and version (#9383)

* history and version

* Packaging update of azure-mgmt-storage

Co-authored-by: Azure SDK Bot <[email protected]>

* [textanalytics] add azure/__init__.py to manifest.in for nspkg (#9380)

* add azure/__init__ to manifest.in for nspkg

* edit nspkg setup

* revert nspkg changes

* Add tracing_attributes to tracing decorator (#9297)

* Add tracing_attributes to tracing decorator

* Add tests

* Fix request_id tests

* Add attribute tests for tracing decorator

* Blackened

* Improve tests

* Change tracing_attributes to kwargs

* Generated from 5c7e7c408cb77bf5896658ba9556cf9d841d6dae (#9189)

fixing build failure for go sdk

* corrected container registry + history and version (#9392)

* corrected container registry + history and version

* Packaging update of azure-mgmt-containerregistry

Co-authored-by: Azure SDK Bot <[email protected]>

* Update README.md

* Restore eventhub dependency tentatively to b6 (#9384)

* Generated from 2b995fff1a9984825fad215fcb4e317af1a20bf5 (#9237)

Update netapp.json

* Generated from 4bc9f1d4e2d95c6b43e61e59740aae7a79241e3d (#9391)

add the required property on inputschemamapping

* [AutoPR cognitiveservices/data-plane/*] Updating several cognitive services (#3649)

* Generated from cf8d4b7ee4befc39b377d86669411e557618edf4

Add cognitiveservices\data-plane\NewsSearch\readme.typescript.md

* Packaging update of azure-cognitiveservices-search-newssearch

* Packaging update of azure-cognitiveservices-search-newssearch

* Packaging update of azure-cognitiveservices-language-spellcheck

* regeneared search services

* Packaging update of azure-cognitiveservices-search-autosuggest

* Packaging update of azure-cognitiveservices-search-videosearch

* Packaging update of azure-cognitiveservices-search-websearch

* Packaging update of azure-cognitiveservices-search-entitysearch

* Packaging update of azure-cognitiveservices-search-customimagesearch

* Packaging update of azure-cognitiveservices-search-customsearch

* fixed test

* updated history and version

* removed change made by accident?

* spacing

* additional test fixes

* try to fix test

* fix test again

* skipping spell test check

* import unittest

* skipping tests

Co-authored-by: Azure Builder <[email protected]>
Co-authored-by: Zim Kalinowski <[email protected]>

* [Event Hubs] Eventhubs update sample and docs (#9407)

* Update release date

* Add starting_position to receive samples

* Add starting_position to code snippet

* Add starting_position to some code snippet

* Docstring update

* Fix pylint error of docstring

* Update docstring, comment from review

* Netapp history and version (#9413)

* history and version

* test

* Packaging update of azure-mgmt-netapp

* Update HISTORY.rst

* Update HISTORY.rst

Co-authored-by: Azure SDK Bot <[email protected]>

* eventgrid - history and version (#9414)

* history and version

* Packaging update of azure-mgmt-eventgrid

* Update HISTORY.rst

* Update HISTORY.rst

Co-authored-by: Azure SDK Bot <[email protected]>

* [Event Hubs] Update checkpoint store dependency on eventhubs to 5.0.0 for release (#9423)

* Small changes on README (#9431)

* remove parallel spinner for tox logs. allow eventhub to run in serial (#9428)

* Expose methods for closing async credential transport sessions (#9090)

* update changelog and release date (#9436)

* OpenTelemetry plugin (#7703)

* Introducing change_context to replace set_current_span

* OpenTelemetry plugin:first pass

* Plug OpenTelemetry in azure-core

* Restore proper settings and tracing tests in azure-core

* Create HttpSpanMixin

* Need opentelemetry-sdk for testing

* WIP core test

* Use stable azure-core

* Support Python 3.8

* OT Readme

* Do not include YAML

* Dependency update

* Update dep

* Add decorator tests in azure-core

* Remove OpenCensus specific tests

* Remove useless OpenCensus tests

* OT testing

* Versionning

* Fix Link for OT

* Fix setup.py

* pylint

* Dependency work

* pylint

* Need Python 3.x at least

* Remove diff with master

* Remove core changes

* Feedback

* Clarify tests

* Update azure-identity history and version (#9437)

* OpenCensus plugin 1.0.0b5 (#9425)

* OpenCensus plugin 1.0.0b5

* Update HISTORY.md

* update version number to 1.2.0 (#9440)

Co-authored-by: Scott Beddall <[email protected]>
Co-authored-by: Peter Marcu <[email protected]>
Co-authored-by: Adam Ling (MSFT) <[email protected]>
Co-authored-by: Krista Pratico <[email protected]>
Co-authored-by: Charles Lowell <[email protected]>
Co-authored-by: iscai-msft <[email protected]>
Co-authored-by: Zim Kalinowski <[email protected]>
Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: openapi-sdkautomation[bot] <37845953+openapi-sdkautomation[bot]@users.noreply.github.com>
Co-authored-by: Laurent Mazuel <[email protected]>
Co-authored-by: Wes Haggard <[email protected]>
Co-authored-by: Yijun Xie <[email protected]>
Co-authored-by: Azure Builder <[email protected]>
xiangyan99 added a commit that referenced this pull request Jan 14, 2020
@xiangyan99 @scbedd
@Petermarcu @yunhaoling @kristapratico @chlowell @iscai-msft @AutorestCI @openapi-sdkautomation @lmazuel @weshaggard @YijunXieMS @azuresdkci
Core add user agent (#9355)
87d2f2a 
* support add_user_agent

* add new line

* fix test case

* updates

* update per review feedback

* fix type

* correct type

* update type

* Get latest master (#9441)

* Index Creation Bugfixes (#9334)

* porting updates for azure-mgmt-web namespaces. updating index creation script to include replace relative links. small updates to package json. updating documentation landing page with appropriate link text and location.
* updating 404ing reference to cspack tool, resolving 404ing link to powershell cmdlet, removing pymodule index from index, remove links to some standard sphinx constructs that won't stick around anymore, override reference to 404ing roboto-slab font.

* Run Tox Test Environments in Parallel (#9034)

* take advantage of improvements to tox-monorepo, plumb through tox environments in parallel
* update tox.ini to only use `envtmpdir` instead of `distdir` for temp work.
* disabling coverage in sdist environment
* cogservices textanalytics uses core
* changing the whl to use envtmpdir
* replacing dev_requirements during CI build. this should  mitigate CI failures due to parallelization of install during tox runs
* ignoring the .tox directory to stop pytest discovery errors during collection

* Update issue templates (#9353)

Add issue and feature templates, mirroring the pattern used in Azure JS SDKs.

* [EventHubs] Sample, Doc, Test improvement (#9349)

* revert back and improve comments

* remove livetest mark for tests which are not livetest

* update docstring and readme

* readme and sample improvement

* update migration guide

* minor update on history

* review update

* small fix of samples

* [textanalytics] update history.md (#9368)

* update history for TA preview release

* add note to track1

* formatting

* add contributing for readme (#9369)

* Remove enter and exit from async context managers (#9313)

* Adding testutils to defined packages within azure-common (#9365)

* adding testutils to defined packages within azure-common
* moving testutils out of azure-common and into azure-sdk-tools

* updated version and changelog (#9373)

* [textanalytics] Add azure-ai-nspkg to artifacts in ci.yml (#9371)

* update ci.yml to include nspkg

* newline

* add setup.cfg to nspkg

* readme tweaks and fix innererror code returned

* clean up temp file after run (#9356)

* clean up temp file after run

* use tempfile

* Fix certs readme (#9370)

* Improve DefaultAzureCredential docstring and error message (#9376)

* Categorize Text Analytics (#9377)

* categorize textanalytics

* Manually generate iothub (#9378)

* multi-api package

* history and version

* Packaging update of azure-mgmt-iothub

* rerecorded tests

Co-authored-by: Azure SDK Bot <[email protected]>

* Restapi auto databoxedge/resource manager (#9367)

* [AutoPR databoxedge/resource-manager] chore: jsonfmt databoxedge (#7361)

* Generated from 8b79ae248e5093f824f1c3c0df81a20a3b3cd4de

chore: jsonfmt databoxedge

* Packaging update of azure-mgmt-databoxedge

* [AutoPR databoxedge/resource-manager]   Add new version of API with object store, sku selection and arm lite user (#8735)

* Generated from b9b42fd9797234b52ae3f05f93711067560d2544

Fixed comments on Sku API

* Generated from b502551c6a94008e85be6c3a666f7f3edfc2b67f

Removed unused property

* Generated from b288943f733062a70176e972eeb92e46c85fa6a1

"Changed operationId for skus API"

* databoxedge generated multi-api

* history and version

* Packaging update of azure-mgmt-databoxedge

* added pipeline

Co-authored-by: Azure SDK Bot <[email protected]>

* [EventHubs] Comment, Test Updates  (#9366)

* update docstring

* await task to prevent potential seg fault

* review update

* PM review update

* review update

* review update

* minor improvement

* minor improvement

* Generated from 1998d0a6549d02cceed76351dbbbfc131382fa80 (#9382)

Adding all files, examples (removed the readonly from encryption services for table and queue)

* [AutoPR storagesync/resource-manager] Microsoft.StorageSync 2019-06-01 API version (#7846)

* Generated from a2749eec38ba462433316a22161df798373732f2

Updates API version in new specs and examples

* Generated from 565b2bebd47ef8fc6b4eba421fe5cc863d15e062

Update StorageSync 2019-06-01 protocol with cloud tiering and recall status (#7161)

* introduce cloud tiering and recall status

* fix validation errors

* Additional validation fixes

* Packaging update of azure-mgmt-storagesync

* Packaging update of azure-mgmt-storagesync

* history and version

Co-authored-by: Zim Kalinowski <[email protected]>

* history and version (#9383)

* history and version

* Packaging update of azure-mgmt-storage

Co-authored-by: Azure SDK Bot <[email protected]>

* [textanalytics] add azure/__init__.py to manifest.in for nspkg (#9380)

* add azure/__init__ to manifest.in for nspkg

* edit nspkg setup

* revert nspkg changes

* Add tracing_attributes to tracing decorator (#9297)

* Add tracing_attributes to tracing decorator

* Add tests

* Fix request_id tests

* Add attribute tests for tracing decorator

* Blackened

* Improve tests

* Change tracing_attributes to kwargs

* Generated from 5c7e7c408cb77bf5896658ba9556cf9d841d6dae (#9189)

fixing build failure for go sdk

* corrected container registry + history and version (#9392)

* corrected container registry + history and version

* Packaging update of azure-mgmt-containerregistry

Co-authored-by: Azure SDK Bot <[email protected]>

* Update README.md

* Restore eventhub dependency tentatively to b6 (#9384)

* Generated from 2b995fff1a9984825fad215fcb4e317af1a20bf5 (#9237)

Update netapp.json

* Generated from 4bc9f1d4e2d95c6b43e61e59740aae7a79241e3d (#9391)

add the required property on inputschemamapping

* [AutoPR cognitiveservices/data-plane/*] Updating several cognitive services (#3649)

* Generated from cf8d4b7ee4befc39b377d86669411e557618edf4

Add cognitiveservices\data-plane\NewsSearch\readme.typescript.md

* Packaging update of azure-cognitiveservices-search-newssearch

* Packaging update of azure-cognitiveservices-search-newssearch

* Packaging update of azure-cognitiveservices-language-spellcheck

* regeneared search services

* Packaging update of azure-cognitiveservices-search-autosuggest

* Packaging update of azure-cognitiveservices-search-videosearch

* Packaging update of azure-cognitiveservices-search-websearch

* Packaging update of azure-cognitiveservices-search-entitysearch

* Packaging update of azure-cognitiveservices-search-customimagesearch

* Packaging update of azure-cognitiveservices-search-customsearch

* fixed test

* updated history and version

* removed change made by accident?

* spacing

* additional test fixes

* try to fix test

* fix test again

* skipping spell test check

* import unittest

* skipping tests

Co-authored-by: Azure Builder <[email protected]>
Co-authored-by: Zim Kalinowski <[email protected]>

* [Event Hubs] Eventhubs update sample and docs (#9407)

* Update release date

* Add starting_position to receive samples

* Add starting_position to code snippet

* Add starting_position to some code snippet

* Docstring update

* Fix pylint error of docstring

* Update docstring, comment from review

* Netapp history and version (#9413)

* history and version

* test

* Packaging update of azure-mgmt-netapp

* Update HISTORY.rst

* Update HISTORY.rst

Co-authored-by: Azure SDK Bot <[email protected]>

* eventgrid - history and version (#9414)

* history and version

* Packaging update of azure-mgmt-eventgrid

* Update HISTORY.rst

* Update HISTORY.rst

Co-authored-by: Azure SDK Bot <[email protected]>

* [Event Hubs] Update checkpoint store dependency on eventhubs to 5.0.0 for release (#9423)

* Small changes on README (#9431)

* remove parallel spinner for tox logs. allow eventhub to run in serial (#9428)

* Expose methods for closing async credential transport sessions (#9090)

* update changelog and release date (#9436)

* OpenTelemetry plugin (#7703)

* Introducing change_context to replace set_current_span

* OpenTelemetry plugin:first pass

* Plug OpenTelemetry in azure-core

* Restore proper settings and tracing tests in azure-core

* Create HttpSpanMixin

* Need opentelemetry-sdk for testing

* WIP core test

* Use stable azure-core

* Support Python 3.8

* OT Readme

* Do not include YAML

* Dependency update

* Update dep

* Add decorator tests in azure-core

* Remove OpenCensus specific tests

* Remove useless OpenCensus tests

* OT testing

* Versionning

* Fix Link for OT

* Fix setup.py

* pylint

* Dependency work

* pylint

* Need Python 3.x at least

* Remove diff with master

* Remove core changes

* Feedback

* Clarify tests

* Update azure-identity history and version (#9437)

* OpenCensus plugin 1.0.0b5 (#9425)

* OpenCensus plugin 1.0.0b5

* Update HISTORY.md

* update version number to 1.2.0 (#9440)

Co-authored-by: Scott Beddall <[email protected]>
Co-authored-by: Peter Marcu <[email protected]>
Co-authored-by: Adam Ling (MSFT) <[email protected]>
Co-authored-by: Krista Pratico <[email protected]>
Co-authored-by: Charles Lowell <[email protected]>
Co-authored-by: iscai-msft <[email protected]>
Co-authored-by: Zim Kalinowski <[email protected]>
Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: openapi-sdkautomation[bot] <37845953+openapi-sdkautomation[bot]@users.noreply.github.com>
Co-authored-by: Laurent Mazuel <[email protected]>
Co-authored-by: Wes Haggard <[email protected]>
Co-authored-by: Yijun Xie <[email protected]>
Co-authored-by: Azure Builder <[email protected]>

* add changelog

Co-authored-by: Scott Beddall <[email protected]>
Co-authored-by: Peter Marcu <[email protected]>
Co-authored-by: Adam Ling (MSFT) <[email protected]>
Co-authored-by: Krista Pratico <[email protected]>
Co-authored-by: Charles Lowell <[email protected]>
Co-authored-by: iscai-msft <[email protected]>
Co-authored-by: Zim Kalinowski <[email protected]>
Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: openapi-sdkautomation[bot] <37845953+openapi-sdkautomation[bot]@users.noreply.github.com>
Co-authored-by: Laurent Mazuel <[email protected]>
Co-authored-by: Wes Haggard <[email protected]>
Co-authored-by: Yijun Xie <[email protected]>
Co-authored-by: Azure Builder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryevdv bryevdv bryevdv approved these changes

@lmazuel lmazuel Awaiting requested review from lmazuel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xiangyan99 @bryevdv