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

Make providers configurable, add config file for easier configuration #15

Merged
merged 13 commits into from
Sep 30, 2024

Conversation

arnasbr
Copy link
Contributor

@arnasbr arnasbr commented Sep 25, 2024

Allowing the user to easily configure which providers should be compared to TravelTime.

src/traveltime_google_comparison/requests/here_handler.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@pawel-wroniszewski
Copy link
Collaborator

In general LGTM, although I agree that a config file would be helpful with such an amount of options.

@arnasbr arnasbr marked this pull request as draft September 26, 2024 08:43
Comment on lines +121 to +124
def parse_config(file_path: str):
with open(file_path, "r") as file: # letting it crash if this fails
content = file.read()
return parse_json_to_providers(content)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose I could add some custom error handling here, but we want to let it crash anyways, so didn't seem useful

@arnasbr arnasbr changed the title Make providers configurable Make providers configurable, add config file for easier configuration Sep 26, 2024
@arnasbr arnasbr marked this pull request as ready for review September 26, 2024 13:14
@arnasbr
Copy link
Contributor Author

arnasbr commented Sep 26, 2024

Added a json config file, moved provider, max_rpm and credentials configuration there

Copy link
Collaborator

@pawel-wroniszewski pawel-wroniszewski left a comment

Choose a reason for hiding this comment

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

LGTM

@arnasbr arnasbr merged commit 6c64c07 into master Sep 30, 2024
5 checks passed
@arnasbr arnasbr deleted the make-providers-pickable branch September 30, 2024 11:01
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.

3 participants