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

Parse & add the missing API details #287

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

MaziyarMK
Copy link

  • offer request owner (airline) logo
  • offer request slice segment stops

@MaziyarMK MaziyarMK requested a review from a team as a code owner June 10, 2023 21:50
Copy link

@ulissesalmeida ulissesalmeida left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me.

I added a few comments to keep the file consistent.

duffel_api/models/offer.py Show resolved Hide resolved
duffel_api/models/offer.py Outdated Show resolved Hide resolved
Copy link

@ulissesalmeida ulissesalmeida left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jesse-c jesse-c left a comment

Choose a reason for hiding this comment

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

LGTM aside from needing the test fixtures updated! Thanks for your work on this, @MaziyarMK.

@@ -11,6 +11,8 @@ class Airline:
id: str
name: str
iata_code: Optional[str]
logo_lockup_url: Optional[str]
Copy link
Contributor

@jesse-c jesse-c Jun 13, 2023

Choose a reason for hiding this comment

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

test: Could you please add examples of these to the test fixture? [1][2]

[1]

{
"iata_code": "BA",
"id": "aln_00001876aqC8c5umZmrRds",
"name": "British Airways"
}

[2]

    {
      "iata_code": "BA",
      "id": "aln_00001876aqC8c5umZmrRds",
      "name": "British Airways",
      "logo_lockup_url": "https://assets.duffel.com/img/airlines/for-light-background/full-color-lockup/BA.svg",
      "logo_symbol_url": "https://assets.duffel.com/img/airlines/for-light-background/full-color-logo/BA.svg"
    }

"""Additional segment-specific information about the stops, if any, included in the segment"""

id: str
duration: str
Copy link
Contributor

Choose a reason for hiding this comment

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

test: Similar request here to add some stops to the test fixture [1].

[1]

{
"aircraft": {
"iata_code": "380",
"id": "arc_00009UhD4ongolulWd91Ky",
"name": "Airbus Industries A380"
},
"arriving_at": "2020-06-13T16:38:02",
"departing_at": "2020-06-13T16:38:02",
"destination": {
"city": {
"iata_code": "NYC",
"iata_country_code": "US",
"id": "cit_nyc_us",
"name": "New York"
},
"city_name": "New York",
"iata_code": "JFK",
"iata_country_code": "US",
"icao_code": "KJFK",
"id": "arp_jfk_us",
"latitude": 40.640556,
"longitude": -73.778519,
"name": "John F. Kennedy International Airport",
"time_zone": "America/New_York"
},
"destination_terminal": "5",
"distance": "424.2",
"duration": "PT02H26M",
"id": "seg_00009htYpSCXrwaB9Dn456",
"marketing_carrier": {
"iata_code": "BA",
"id": "aln_00001876aqC8c5umZmrRds",
"name": "British Airways"
},
"marketing_carrier_flight_number": "1234",
"operating_carrier": {
"iata_code": "BA",
"id": "aln_00001876aqC8c5umZmrRds",
"name": "British Airways"
},
"operating_carrier_flight_number": "4321",
"origin": {
"city": {
"iata_code": "LON",
"iata_country_code": "GB",
"id": "cit_lon_gb",
"name": "London"
},
"city_name": "London",
"iata_code": "LHR",
"iata_country_code": "GB",
"icao_code": "EGLL",
"id": "arp_lhr_gb",
"latitude": 64.068865,
"longitude": -141.951519,
"name": "Heathrow",
"time_zone": "Europe/London"
},
"origin_terminal": "B",
"passengers": [
{
"baggages": [
{
"quantity": 1,
"type": "checked"
}
],
"cabin_class": "economy",
"cabin_class_marketing_name": "Economy Basic",
"fare_basis_code": "OXZ0RO",
"passenger_id": "passenger_0"
}
]
}

@@ -11,6 +11,8 @@ class Airline:
id: str
name: str
iata_code: Optional[str]
logo_lockup_url: Optional[str]
Copy link
Contributor

Choose a reason for hiding this comment

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

future: We need to add conditions_of_carriage_url [1] as well.

[1] https://duffel.com/docs/api/v1/airlines/schema#airlines-schema-conditions-of-carriage-url.

@@ -221,6 +221,32 @@ def from_json(cls, json: dict):
)


@dataclass
class OfferSliceSegmentStop:
"""Additional segment-specific information about the stops, if any, included in the segment"""
Copy link
Contributor

@jesse-c jesse-c Jun 13, 2023

Choose a reason for hiding this comment

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

fix: This line is too long [1]. You can fix it by running tox -e linting or flake8 locally.

[1] https://github.com/duffelhq/duffel-api-python/actions/runs/5245627223/jobs/9476358835?pr=287

Comment on lines +239 to +244
arriving_at=get_and_transform(
json, "arriving_at", parse_datetime
),
departing_at=get_and_transform(
json, "departing_at", parse_datetime
),
Copy link
Contributor

Choose a reason for hiding this comment

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

fix: The reason for the static analysis (pyright) failures is that this helper function, get_and_transform, is for when values are optional.

Those 2 aren't, so you can parse them directly.

Suggested change
arriving_at=get_and_transform(
json, "arriving_at", parse_datetime
),
departing_at=get_and_transform(
json, "departing_at", parse_datetime
),
arriving_at=parse_datetime(json["arriving_at"]),
departing_at=parse_datetime(json["departing_at"]),

Choose a reason for hiding this comment

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

Ahh sorry, it was my bad

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.

3 participants