Skip to content
This repository has been archived by the owner on Sep 17, 2024. It is now read-only.

Improvement: if the Debug true disable the verify ssl in fetch content, That form is better to run local, I think :) #27

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Improvement: if the Debug true disable the verify ssl in fetch content, That form is better to run local, I think :) #27

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1028c4c
Improvement: if the Debug true disable the verify ssl in fetch conten…
elinaldosoft Oct 12, 2018
c5aea70
Add _blank to open content on external page
elinaldosoft Oct 12, 2018
db3740f
Refactory: disable verify ssl
elinaldosoft Oct 12, 2018
21aad07
fixbug: format file by black
elinaldosoft Oct 12, 2018
1698e18
Add test and fixbug disable ssl, logic was inverted
elinaldosoft Oct 12, 2018
a98bfbc
Fixbug: format file by black
elinaldosoft Oct 12, 2018
86e4294
Remove target blank in PR wrong
elinaldosoft Oct 15, 2018
fde87d9
Remove test with depends external
elinaldosoft Oct 15, 2018
d64a255
Refactory: add option very_ssl false in cliente session if debug true
elinaldosoft Oct 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions victims/data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
CASE_LABELS,
STORIES_SPREADSHEET_GID,
STORY_LABELS,
DEBUG,
)


Expand All @@ -24,6 +25,10 @@ def round_robin(*iterables):

class Data:
async def fetch(self, session, url):
if DEBUG:
session = aiohttp.ClientSession(
connector=aiohttp.TCPConnector(verify_ssl=False)
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is not the proper place to handle this kwarg. The session is defined a few lines above and you're overwriting it. Beyond that now there's two different places defining sessions for the same request, what is a bit confusing. I'm gonna suggest something different around line 40;

     async def fetch_spreadsheets(self):
        urls = (…)
        kwargs = {'loop': asyncio.get_event_loop()}
        if DEBUG:
            kwargs = {'connector': aiohttp.TCPConnector(verify_ssl=False)}
        
        async with aiohttp.ClientSession(**kwargs) as session:
            …

async with session.get(url) as response:
return await response.read()

Expand Down
2 changes: 1 addition & 1 deletion victims/templates/home.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
</div><!-- end .meta -->

<h2>
<a class="no-underline" href="{{ case.main_story.url }}">{{ case.main_story.title|e }}</a>
<a class="no-underline" href="{{ case.main_story.url }}" target="_blank">{{ case.main_story.title|e }}</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't belong in this PR. Please, check #9.

</h2>

<h3 class="grey-text">
Expand Down
7 changes: 7 additions & 0 deletions victims/tests/test_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,10 @@ def test_data_cases_property(mocker):
assert len(cases) == 5
cases[0].when == date(2018, 10, 8)
cases[-1].when == date(2018, 10, 3)


def test_data_cases_property_by_request():
data = Data()
loop = asyncio.get_event_loop()
cases = loop.run_until_complete(data.cases())
assert len(cases) == 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not advisable. It makes tests depends on external connection and availability, and it also depends on envvar settings (have you seen it's failing on Travis? We use the production stylesheet there, probably that's why it's failing).