-
Notifications
You must be signed in to change notification settings - Fork 54
AV2 - Misc functions #986
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
AV2 - Misc functions #986
Conversation
✅ Deploy Preview for neo4j-graph-data-science-client canceled.
|
graphdatascience/procedure_surface/api/catalog/relationships_endpoints.py
Outdated
Show resolved
Hide resolved
graphdatascience/procedure_surface/api/catalog/relationships_endpoints.py
Outdated
Show resolved
Hide resolved
graphdatascience/procedure_surface/api/catalog/relationships_endpoints.py
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| def test_defaults_set_and_list(config_endpoints: ConfigArrowEndpoints) -> None: | ||
| config_endpoints.defaults.set("test.key", 6) |
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.
we should transform snake to camel case for the defaults as the python API shows the snake casing
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.
I will only do it for the set endpoints I think
1624c15 to
14531e1
Compare
| config_endpoints.defaults.set("test.specific.key", "specific_value") | ||
|
|
||
| specific_defaults = config_endpoints.defaults.list(key="test.specific.key") | ||
|
|
||
| assert specific_defaults == {"test.specific.key": "specific_value"} | ||
|
|
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.
NIT: you could test a snake_case field here in these tests
| yield job_id | ||
|
|
||
|
|
||
| @pytest.mark.skip(reason="Enable when we figure out how to retain jobs") |
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.
its going against the gds_plugin container here.
you can set gds.progress_tracking_retention_period in the test container to lets say 1H to keep it.
so for the docker setting in t would be `gds_progress__tracking__retention__period
FlorentinD
left a comment
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.
only two smaller comments on the testing
ref GDSA-75