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

Init Handimap connector #3911

Merged
merged 20 commits into from
Feb 1, 2023
Merged

Init Handimap connector #3911

merged 20 commits into from
Feb 1, 2023

Conversation

azime
Copy link
Contributor

@azime azime commented Jan 18, 2023

No description provided.

@@ -495,6 +495,20 @@ def __init__(self):
hidden=True,
help="Here, Active or not the realtime traffic information (True/False)",
)
parser_get.add_argument(
Copy link
Member

Choose a reason for hiding this comment

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

I think we should stop adding parameters _*_language every time we add a new connector...

let's refactorise that later...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

):
if mode != "walking":
logging.getLogger(__name__).error('Handimap, mode {} not implemented'.format(mode))
raise InvalidArguments('Handimap, mode {} not implemented'.format(mode))
Copy link
Member

Choose a reason for hiding this comment

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

👍
can also check this in the constructor plz?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

self.auth = (username, password)
self.headers = {"Content-Type": "application/json", "Accept": "application/json"}
self.timeout = timeout
self.modes = modes
Copy link
Member

Choose a reason for hiding this comment

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

can you check if walking is the only mode defined? and raise and exception if it's not the case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,336 @@
# Copyright (c) 2001-2022, Hove and/or its affiliates. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Copyright (c) 2001-2022, Hove and/or its affiliates. All rights reserved.
# Copyright (c) 2001-2023, Hove and/or its affiliates. All rights reserved.

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

try:
return Languages[language].value
except KeyError:
self.log.error('Handimap parameters language={} not exist - force to french'.format(language))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.log.error('Handimap parameters language={} not exist - force to french'.format(language))
self.log.error('Handimap parameter language={} is not a valid parameter - language is set to french by default'.format(language))

Copy link
Member

Choose a reason for hiding this comment

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

why is the default value french?

post_data,
)
self.check_response(response)
return ujson.loads(response.text)
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 recommend you to add a try-catch around ujson.loads in case of non utf-8 characters...


origin = make_pt_object(type_pb2.ADDRESS, lon=-1.6761174, lat=48.1002462, uri='HandimapStart')
destination = make_pt_object(type_pb2.ADDRESS, lon=-1.6740057, lat=48.097592, uri='HandimapEnd')
fallback_extremity = PeriodExtremity(str_to_time_stamp('20220503T060000'), True)
Copy link
Member

Choose a reason for hiding this comment

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

can you test with PeriodExtremity(str_to_time_stamp('20220503T060000'), False) plz

if index == len(handimap_trip['legs']) - 1:
section.destination.CopyFrom(pt_object_destination)

section.street_network.duration = section.duration
Copy link
Member

Choose a reason for hiding this comment

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

bad indentation?

section.id = 'section_{}'.format(index)
section.length = int(round(kilometers_to_meters(handimap_leg["summary"]['length'])))

if index == 0:
Copy link
Member

Choose a reason for hiding this comment

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

to avoid branches (if), you can do operations to the specific items at the end..

Comment on lines 289 to 291
section.street_network.duration = section.duration
section.street_network.length = section.length
section.street_network.mode = response_pb2.Walking
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
section.street_network.duration = section.duration
section.street_network.length = section.length
section.street_network.mode = response_pb2.Walking
section.street_network.duration = section.duration
section.street_network.length = section.length
section.street_network.mode = response_pb2.Walking

Copy link
Member

@xlqian xlqian left a comment

Choose a reason for hiding this comment

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

👍 GG!!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

86.6% 86.6% Coverage
0.0% 0.0% Duplication

@azime azime merged commit 17d3828 into dev Feb 1, 2023
@azime azime deleted the handimap_connector branch February 1, 2023 13:15
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