PB-974: Migration from trias to ojp for transport stop requests #4487

rebert · 2024-11-12T08:10:45Z

No description provided.

hansmannj · 2024-11-18T08:02:22Z

Guess this has been moved to another PR by @rebert during deploy, when changing the base branch.

rebert · 2024-11-18T08:37:35Z

yes, it is not possible to merge develop towards master with open prs on develop

hansmannj · 2024-11-18T09:26:03Z

yes, it is not possible to merge develop towards master with open prs on develop

Yes, sorry for bugging the deploy process and thanks for moving the commmits 🙏

ltshb

PR title should be updated and CI is not passing

hansmannj · 2024-11-18T13:26:37Z

PR title should be updated and CI is not passing

Yes, need to adapt CI to use the new secrets, will hopefully find some time for that, soon.
Can you in the meantime have a look at the unit tests please? Is the mocking what you asked for in the comments in the old PR? One second please, just updating the tests. In one case, currently way too much is mocked.
Will take some time, I'll give you a ping @ltshb once I am ready.
Sorry, only noticed this now. Guess I'll only mock "send_post()"

ltshb · 2024-11-19T08:41:40Z

PR title should be updated and CI is not passing

Yes, need to adapt CI to use the new secrets, will hopefully find some time for that, soon. Can you in the meantime have a look at the unit tests please? Is the mocking what you asked for in the comments in the old PR? One second please, just updating the tests. In one case, currently way too much is mocked. Will take some time, I'll give you a ping @ltshb once I am ready. Sorry, only noticed this now. Guess I'll only mock "send_post()"

Well by mocking we should not need any credential because no access to the real service should be done, unit test should not do any access to the service so they should pass even if the service is down

hansmannj · 2024-11-19T13:48:08Z

tests/integration/test_stationboard.py

+        with self.assertRaises(opentransapi.OpenTransNoStationException):
+            api.get_departures("invalid_id")
+
+    @patch.object(opentransapi.OpenTrans, "send_post")


I guess this test can as well be deleted.
Here we only test, if the side_effect() works basically 😉

hansmannj · 2024-11-19T13:52:32Z

@rebert @ltshb : Guess now this PR is ready for review.
I tried to only mock "send_post" and condensed the tests to those that seem to make sense.
There's one left, that can be deleted, too, as it only tests an exception we provoke using side_effects().
Even better: Now the CI does not need any secrets any more, since we are mocking all requests :-)

rebert

lgtm. but I'm not allowed to approve :-)

tests/integration/test_stationboard.py

ltshb

Unittest mocking needs adaptation as well as API key need to be removed from CI

hansmannj · 2024-11-21T08:29:33Z

Please re-check the PR.
Will prepare another PR for removing the old secrets from CI soon

ltshb

There is still some issues with the unittest in the CI

chsdi/lib/opentransapi/opentransapi.py

tests/integration/test_stationboard.py

hansmannj · 2024-11-22T07:58:31Z

There is still some issues with the unittest in the CI

Thanks! Forgot to push the updated Pipfile(.lock), should now be fixed

ltshb

Code looks good but still have an issue with the CI

rebert · 2024-12-02T07:36:27Z

lgtm too. But I would rebase and fix the codebuild error.

tests/integration/test_stationboard.py

The API we use to get time table data for open transport stops will be decomissioned soon. Hence we migrate to OJP 2.0. The requests to the API and the parsing of the response needed some adaptions for that. Furthermore, the OJP 2.0 API's response sometimes has a strange timestamp format, there are sometimes 7 instead of the expexted 6 digits for the milliseconds. Hence a try except was used to safely convert to the expected format in that cases. Also some minor code clean-up was done.

…eleted superfluous comments

Originally there were separate test files for opentransapi and the stationboard. This is now again the case, also several test cases got lost, which were re-added again. Also added dependencies in order to fix TypeError: __init__() got an unexpected keyword argument 'management'

hansmannj

LGTM, but I am the wrong one to approve, since I cannot approve my own code 😉

github-actions bot added the feature label Nov 12, 2024

ltclm changed the base branch from master to develop-2024-12-18 November 13, 2024 13:08

github-actions bot added this to the 2024-12-18 milestone Nov 13, 2024

hansmannj requested a review from ltshb November 18, 2024 08:01

rebert marked this pull request as ready for review November 18, 2024 08:31

hansmannj changed the base branch from develop-2024-12-18 to master November 18, 2024 09:55

github-actions bot removed this from the 2024-12-18 milestone Nov 18, 2024

ltshb reviewed Nov 18, 2024

View reviewed changes

hansmannj changed the title ~~Feat pb 974 migration from trias to ojp for transport stop requests~~ PB-974 migration from trias to ojp for transport stop requests Nov 18, 2024

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from 054d38c to 75011f8 Compare November 19, 2024 13:47

hansmannj reviewed Nov 19, 2024

View reviewed changes

hansmannj requested a review from ltshb November 19, 2024 14:14

rebert commented Nov 19, 2024

View reviewed changes

ltshb reviewed Nov 20, 2024

View reviewed changes

tests/integration/test_stationboard.py Outdated Show resolved Hide resolved

ltshb requested changes Nov 20, 2024

View reviewed changes

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch 2 times, most recently from 6170a0c to 80520a9 Compare November 20, 2024 17:57

hansmannj requested a review from ltshb November 21, 2024 08:23

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from 80520a9 to c8fb3c4 Compare November 21, 2024 10:24

ltshb reviewed Nov 21, 2024

View reviewed changes

chsdi/lib/opentransapi/opentransapi.py Outdated Show resolved Hide resolved

tests/integration/test_stationboard.py Outdated Show resolved Hide resolved

tests/integration/test_stationboard.py Outdated Show resolved Hide resolved

hansmannj requested a review from ltshb November 21, 2024 14:21

ltshb reviewed Nov 25, 2024

View reviewed changes

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from 5de9dbf to d1e09e3 Compare December 2, 2024 20:24

hansmannj reviewed Dec 2, 2024

View reviewed changes

tests/integration/test_stationboard.py Outdated Show resolved Hide resolved

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from d1e09e3 to 1ecefdc Compare December 2, 2024 20:30

ltshb changed the title ~~PB-974 migration from trias to ojp for transport stop requests~~ PB-974: Migration from trias to ojp for transport stop requests Dec 3, 2024

hansmannj added 7 commits December 3, 2024 18:33

PB-974: mocking calls to external OJP 2.0 API in unit tests

6c83f2a

PB-974: using OJP 2.0 URL form env var

9d489e0

PB-974: strip any non needed whitespaces from the payload

b3a4c8a

PB-974: mocking requests directly, not the send_post method

fdf9fc4

PB-974: mf-chsdi3: re-using functions, so no redundance is needed | d…

d381657

…eleted superfluous comments

PB-974: added missing dependency for requests-mock

155348e

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from 1ecefdc to 574db1a Compare December 4, 2024 10:06

hansmannj marked this pull request as draft December 4, 2024 10:07

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from 574db1a to a15b6b2 Compare December 5, 2024 12:08

hansmannj force-pushed the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch from a15b6b2 to 57444a6 Compare December 5, 2024 14:16

hansmannj added 2 commits December 5, 2024 15:18

PB_974: adapted dummy API URL in config files

d7cbad8

PB-974: unpin new dependencies

e550d59

hansmannj marked this pull request as ready for review December 5, 2024 14:27

hansmannj requested a review from ltshb December 5, 2024 14:27

PB-974: added comment on not changing the value of the RequestorRef

57b5c86

hansmannj approved these changes Dec 5, 2024

View reviewed changes

ltshb approved these changes Dec 9, 2024

View reviewed changes

hansmannj merged commit 3ef3f37 into master Dec 9, 2024
5 checks passed

hansmannj deleted the feat_PB-974_migration_from_trias_to_ojp_for_transport_stop_requests branch December 9, 2024 12:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PB-974: Migration from trias to ojp for transport stop requests #4487

PB-974: Migration from trias to ojp for transport stop requests #4487

rebert commented Nov 12, 2024

hansmannj commented Nov 18, 2024

rebert commented Nov 18, 2024

hansmannj commented Nov 18, 2024

ltshb left a comment

hansmannj commented Nov 18, 2024 •

edited

Loading

ltshb commented Nov 19, 2024

hansmannj Nov 19, 2024 •

edited

Loading

hansmannj commented Nov 19, 2024 •

edited

Loading

rebert left a comment

ltshb left a comment

hansmannj commented Nov 21, 2024

ltshb left a comment

hansmannj commented Nov 22, 2024

ltshb left a comment

rebert commented Dec 2, 2024

hansmannj left a comment

PB-974: Migration from trias to ojp for transport stop requests #4487

PB-974: Migration from trias to ojp for transport stop requests #4487

Conversation

rebert commented Nov 12, 2024

hansmannj commented Nov 18, 2024

rebert commented Nov 18, 2024

hansmannj commented Nov 18, 2024

ltshb left a comment

Choose a reason for hiding this comment

hansmannj commented Nov 18, 2024 • edited Loading

ltshb commented Nov 19, 2024

hansmannj Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

hansmannj commented Nov 19, 2024 • edited Loading

rebert left a comment

Choose a reason for hiding this comment

ltshb left a comment

Choose a reason for hiding this comment

hansmannj commented Nov 21, 2024

ltshb left a comment

Choose a reason for hiding this comment

hansmannj commented Nov 22, 2024

ltshb left a comment

Choose a reason for hiding this comment

rebert commented Dec 2, 2024

hansmannj left a comment

Choose a reason for hiding this comment

hansmannj commented Nov 18, 2024 •

edited

Loading

hansmannj Nov 19, 2024 •

edited

Loading

hansmannj commented Nov 19, 2024 •

edited

Loading