FOGL-1127 - Fetch support bundle functionality along with unit tests #614

ashish-jabble · 2018-03-01T12:01:35Z

Coverage 89%

35 statements   31 run || 4 missing || 0 excluded

Tests O/P

collected 10 items                                                                                                                            

services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle[pyloop-data0-expected_content0-2] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle[pyloop-data1-expected_content1-1] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle[pyloop-data2-expected_content2-0] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle[pyloop-data3-expected_content3-0] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle_by_name[pyloop] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle_by_name_not_found[pyloop-data0-xsupport-180301-133523.tar.gz] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle_by_name_not_found[pyloop-data1-support-180301-133523.tar.gz] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle_by_name_bad_request[pyloop] PASSED
services/core/api/test_support.py::TestBundleSupport::test_get_support_bundle_by_name_no_dir[pyloop] PASSED
services/core/api/test_support.py::TestBundleSupport::test_create_support_bundle[pyloop] PASSED

=== 10 passed in 0.73 seconds ==

Suite

collected 644 items

=== 639 passed, 5 skipped in 31.75 seconds ====

ashish-jabble · 2018-03-01T12:02:44Z

python/foglamp/services/core/api/support.py

+    if _FOGLAMP_DATA:
+        support_dir = os.path.expanduser(_FOGLAMP_DATA + '/tmp/support')
+    else:
+        support_dir = os.path.expanduser(_FOGLAMP_ROOT + '/data/tmp/support')


@MarkRiddoch Please confirm here, if FOGLAMP_DATA is not set and update the JIRA

Yes, this is correct.

ashish-jabble · 2018-03-01T12:04:35Z

python/foglamp/services/core/api/support.py

+        return web.HTTPBadRequest(reason="Bundle file extension is invalid")
+
+    if not os.path.isdir(_get_support_dir()):
+        raise web.HTTPNotFound(reason="Support bundle directory does not exist")


@MarkRiddoch If directory does not exist? HTTPNOTFOUND or HTTPINTERNALSERVER or nothing to raise

If there is no directory then I think we should just raise a not found error as no support bundles have been created.

praveen-garg · 2018-03-02T09:42:19Z

tests/unit/python/foglamp/services/core/api/test_support.py

+            mockwalk.assert_called_once_with(path)
+
+    # FIXME: issue with URL having colon in it even after encoded with %3A manually or urllib parser
+    async def test_get_support_bundle_by_name(self, client, support_bundles_dir_path):


aio-libs/aiohttp#2670
aio-libs/aiohttp@06ed15e /cc @ashish-jabble

@MarkRiddoch As per specs mentioned in FOGL-1126 (The file will be called support--HH:MM:SS.tar.gz).So I had added tests and example with this naming convention but somehow there is aiohttp bug on v2.3.8 and bug has fixed in 2.3.9 BUT with yarl dep 1.0.0

So if we need to follow the same file naming convention, we need to fix below things

aiohttp dep set to 2.3.9

and yarl dep explicitly set to 1.0.0

Please let me know, should I need to do above steps to make it work? or simplest and easiest way to just change the file naming convention we had in specs and do it like "support-YYMMDD-HH-MM-SS" or "support_YYMMDD_HH_MM_SS" or anything.

I'm not too worried about the exact naming, so rather than force this update let's go for one of the alternate naming conventions. I would suggest support-YYMMDD-HH-MM-SS.

praveen-garg · 2018-03-02T09:43:00Z

python/foglamp/services/core/api/support.py

+
+
+async def fetch_support_bundle_item(request):
+    """


add docstring

amarendra-dianomic

LGTM!

Fetch support bundle functionality along with unit tests

8ceacf0

ashish-jabble requested review from praveen-garg, MarkRiddoch and amarendra-dianomic March 1, 2018 12:01

ashish-jabble commented Mar 1, 2018

View reviewed changes

ashish-jabble added the architectural_review_pending label Mar 1, 2018

praveen-garg reviewed Mar 2, 2018

View reviewed changes

python/foglamp/services/core/api/support.py Outdated

async def fetch_support_bundle_item(request):

"""

Copy link

Contributor

praveen-garg Mar 2, 2018

Choose a reason for hiding this comment

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

add docstring

MarkRiddoch previously approved these changes Mar 2, 2018

View reviewed changes

praveen-garg added architectural_review_done and removed architectural_review_pending labels Mar 5, 2018

file name convention changes

dcc3084

ashish-jabble dismissed MarkRiddoch’s stale review via dcc3084 March 6, 2018 12:09

MarkRiddoch approved these changes Mar 6, 2018

View reviewed changes

amarendra-dianomic approved these changes Mar 6, 2018

View reviewed changes

amarendra-dianomic merged commit 822d9ec into develop Mar 6, 2018

amarendra-dianomic deleted the FOGL-1127 branch March 6, 2018 13:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FOGL-1127 - Fetch support bundle functionality along with unit tests #614

FOGL-1127 - Fetch support bundle functionality along with unit tests #614

ashish-jabble commented Mar 1, 2018

ashish-jabble Mar 1, 2018

MarkRiddoch Mar 2, 2018

ashish-jabble Mar 1, 2018

MarkRiddoch Mar 2, 2018

praveen-garg Mar 2, 2018

ashish-jabble Mar 6, 2018

MarkRiddoch Mar 6, 2018

praveen-garg Mar 2, 2018

amarendra-dianomic left a comment

FOGL-1127 - Fetch support bundle functionality along with unit tests #614

FOGL-1127 - Fetch support bundle functionality along with unit tests #614

Conversation

ashish-jabble commented Mar 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amarendra-dianomic left a comment

Choose a reason for hiding this comment