Skip to content
This repository has been archived by the owner on Aug 25, 2023. It is now read-only.

Restoration daily test fix #9

Merged
merged 8 commits into from
Jul 5, 2018
Merged

Conversation

przemyslaw-jasinski
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jul 4, 2018

Pull Request Test Coverage Report for Build 217

  • 31 of 42 (73.81%) changed or added relevant lines in 3 files are covered.
  • 8 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.6%) to 87.813%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/restore/test/table_restore_invoker.py 5 16 31.25%
Files with Coverage Reduction New Missed Lines %
src/restore/test/restore_test.py 2 90.48%
src/google_cloud_storage_client.py 6 40.74%
Totals Coverage Status
Change from base Build 193: -0.6%
Covered Lines: 1996
Relevant Lines: 2273

💛 - Coveralls

radkomateusz
radkomateusz previously approved these changes Jul 4, 2018
result = RestorationJobStatusService()\
.get_restoration_job(restoration_job_id)

if result["status"]["state"] in "In progress":
Copy link
Member

Choose a reason for hiding this comment

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

Periodical retrieving state using exceptions looks really bad. Could you use simple while loop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

.get_restoration_job(restoration_job_id)
if result["status"]["state"] in "Done":
return result
time.sleep(period)
Copy link
Member

Choose a reason for hiding this comment

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

I would log info, how long sleep will take

@marcin-kolda marcin-kolda merged commit b7ffe98 into master Jul 5, 2018
@marcin-kolda marcin-kolda deleted the restoration_daily_test_fix branch July 5, 2018 11:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants