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

Issues with test function for get tracer method #2184

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Swetna
Copy link
Collaborator

@Swetna Swetna commented Feb 19, 2025

The valid function does not seem to be working the other two work fine. The data is not retaining in the test_db.

@Swetna Swetna requested a review from anth-volk February 19, 2025 01:38
@anth-volk anth-volk removed their assignment Feb 19, 2025
@anth-volk
Copy link
Collaborator

anth-volk commented Feb 19, 2025

A couple notes in general before I jump in:

  1. It's usually bad practice to open changes off of your own local master branch. Instead, keep master separate and only use it to regularly run git rebase origin master to pull in our code, and open your own branches off of that
  2. We ask contributors to clone, not fork, the API, as a few of the GitHub Actions systems we use require cloning in order to pass around environment variables, keys, etc. In the future, could you use a branch off of our repo?

@anth-volk
Copy link
Collaborator

@Swetna Could you remind me how this relates to #2233?

@Swetna
Copy link
Collaborator Author

Swetna commented Mar 4, 2025

Ohh ya sure while testing the function I noticed the function does not seem to be checking if the input parameters are of the valid type or within the valid range for eg: The function did not seem to break when I added the household_id as -100 or when the country_code was not a str but a int value instead. It dealt with those cases by checking if they exist in the database or no and if not then returned a not found value

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