-
Notifications
You must be signed in to change notification settings - Fork 5
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
Dataset endpoints #39
Conversation
- Datasets - Dataset rows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one 👍
src/tests/v1/test_dataset_rows.py
Outdated
@pytest.mark.order(test_order_map["dataset_rows"]["create"]) | ||
def test_dataset_row_create_many(client): | ||
response = client.post( | ||
"/v1/datasets/rows", | ||
json=list(map(lambda x: {**x, "dataset_id": state.dataset["id"]}, dataset_rows_create_payload)) | ||
) | ||
assert response.status_code == status.HTTP_201_CREATED | ||
|
||
|
||
@pytest.mark.order(test_order_map["dataset_rows"]["get_dataset's_all"]) | ||
def test_dataset_row_get_datasets_all(client): | ||
response = client.get(f"/v1/datasets/{state.dataset['id']}/rows") | ||
assert response.status_code == status.HTTP_200_OK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if we can also have tests the non-happy paths
src/tests/v1/test_datasets.py
Outdated
@pytest.mark.order(test_order_map["datasets"]["create"]) | ||
def test_dataset_create(client): | ||
response = client.post( | ||
"/v1/datasets/metadata", | ||
json={**dataset_create_payload, "user_id": state.user["id"]}, | ||
) | ||
|
||
state.dataset = response.json() | ||
assert response.status_code == status.HTTP_201_CREATED | ||
|
||
|
||
@pytest.mark.order(test_order_map["datasets"]["get"]) | ||
def test_dataset_get(client): | ||
response = client.get(f"/v1/datasets/{state.dataset['id']}") | ||
assert response.status_code == status.HTTP_200_OK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
@momegas Please re-review. Had some problem with creating 422 in the endpoints and only got 400... We'll write fail case tests for all endpoints soon! |
return new_dataset | ||
else: | ||
return errors.not_found(f"User with id: {body.__dict__['user_id']} not found") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there something better than dict to use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dict in line 29
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, this is a pydantic datastructure right. There is a .dict()
. Have a look here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I think this is a good start. Should need some improvements as we move forward with the schema and all but it look good now 👍
Endpoints for:
closes: #13