-
Notifications
You must be signed in to change notification settings - Fork 2
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
Sync Datalad-Registry with datalad-usage-dashboard #287
Sync Datalad-Registry with datalad-usage-dashboard #287
Conversation
…ls.usage_dashboard`
With the one defined in `datalad_registry.tasks.utils.usage_dashboar`
With the one defined in `datalad_registry.tasks.utils.usage_dashboar`
For referring to the path of the dataset URLs resource on the DataLad Registry instance relative to the base API endpoint of the instance.
The definition is defined in `datalad_registry.blueprints.api.dataset_urls`
From `datalad_registry.blueprints.api.dataset_urls` to `datalad_registry.blueprints.api`
So that organization is consistent with the definition of `DATASET_URLS_PATH`
mypy does like the default to be just plain str
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #287 +/- ##
==========================================
- Coverage 98.74% 98.73% -0.02%
==========================================
Files 48 50 +2
Lines 2237 2366 +129
==========================================
+ Hits 2209 2336 +127
- Misses 28 30 +2 ☔ View full report in Codecov by Sentry. |
To specify invocation condition
For access to the DB
@@ -631,3 +636,101 @@ def chk_url_to_update( | |||
db.session.commit() | |||
|
|||
return ChkUrlStatus.OK_UPDATED if is_record_updated else ChkUrlStatus.OK_CHK_ONLY | |||
|
|||
|
|||
class FailedSubmission(TypedDict): |
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.
why TypedDict
and not dataclass
es which seems to be more common construct pattern 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.
The reason I used type TypedDict
is because a dictionary displays better in the Flower interface. I don't have fine control of how the result is displayed by Flower. Modifying __str__
or __repr__
didn't work, possibly due to serialization in the transmission from the worker service to the Flower service. Thus, I picked a type that displays more nicely by default.
datalad_registry/tasks/__init__.py
Outdated
|
||
class FailedSubmission(TypedDict): | ||
""" | ||
A TypedDict a failed submission of a repo URL to Datalad-Registry |
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.
not a sentence... not sure if worth stating TypedDict type here instead of purpose only
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.
Yeah, stating TypedDict
here is unnecessary. The docstring has been updated.
@jwodder please review |
@yarikoptic Is it OK to merge now? |
sure... I didn't fully grasp why we need so many changes but if it accomplishes the drill -- let's proceed! |
I think the additional changes you are referring to are those resulted from the refactoring of the |
This PR does the following.
tools.populate.py
so that these models can be used by the aforementioned Celery task as well.responses
will be added as a dependency for testing for the purpose of mocking out therequests
library.Note: Changes of this PR has been applied to the Typhon instance.