-
Notifications
You must be signed in to change notification settings - Fork 78
add migration v13 v14 steps #900
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -352,6 +352,9 @@ invenio alembic upgrade | |||||
|
|
||||||
| Execute the data migration: | ||||||
|
|
||||||
| TODO: `publication-disseration` has to be added to the vocabulary first. either | ||||||
| with a `vocabulary_service.create` or with some another step. | ||||||
|
|
||||||
| ```bash | ||||||
| invenio shell $(find $(pipenv --venv)/lib/*/site-packages/invenio_app_rdm -name migrate_13_0_to_Next.py) | ||||||
| ``` | ||||||
|
|
@@ -396,6 +399,87 @@ The full list of ID changes [can be found here](https://github.com/inveniosoftwa | |||||
|
|
||||||
| If you are not overriding any of these components, you do not need to change anything. | ||||||
|
|
||||||
| #### Marshmallow Context Deprecation | ||||||
|
|
||||||
| Marshmallow has deprecated `self.context` in `marshmallow<4.0.0` and will remove | ||||||
| it with `marshmallow>=4.0.0`. Until now we don't go to `marshmallow>=4.0.0`, | ||||||
| there would be further work necessary, because there are backwards incompatible | ||||||
| changes coming in with the n ew version. | ||||||
|
|
||||||
| The changes are split into 4 groups and they are: | ||||||
|
|
||||||
| the first group is fixed by creating the `ContextVar` `context_schema` in | ||||||
| `marshmallow_utils.context` | ||||||
|
|
||||||
| - `max_number` | ||||||
| - `identity` | ||||||
| - `field_permission_check` | ||||||
| - `request` | ||||||
| - `object_version_id` | ||||||
| - `bucket` | ||||||
| - `multipart` | ||||||
|
|
||||||
| the second group is fixed by moving the parameter into the constructor of the Schema | ||||||
|
|
||||||
| - `doi_all_version` | ||||||
| - `is_parent` | ||||||
| - `record_dict` | ||||||
|
|
||||||
| the third group is fixed by changing the parameter to a class property | ||||||
|
|
||||||
| - `object_key` (`= "ui"`) | ||||||
|
|
||||||
| the fourth group is kept in `self.context` because it never reaches the | ||||||
| marshmallow level, so it doesn't produce a `DeprecationWarning` | ||||||
|
|
||||||
| - `is_self` | ||||||
|
Comment on lines
+402
to
+435
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section doesn't seem to imply any action for an instance operator, so I'd remove it or replace it with a small note in The current text is internal to maintainers. I would create an "Epic" ticket on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my thinking was, that this information could be interesting for people who customize their instance and maybe build something depending of those changes |
||||||
|
|
||||||
| #### utcnow DeprecationWarning | ||||||
|
|
||||||
| the usage of `datetime.datetime.utcnow` is deprecated. InvenioRDM replaces it | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| with `datetime.datetime.now(datetime.timezone.utc)`. This means, we go from a | ||||||
| utc unaware datetime to a utc aware datetime. | ||||||
|
|
||||||
| This change includes the database, where the `db.DateTime` columns are changed | ||||||
| to `db.UTCDateTime` columns. | ||||||
|
|
||||||
| invenio-stats changes the format from `strict_date_hour_minute_second` to | ||||||
| `strict_date_optional_time` in `file-download-v1.json` and | ||||||
| `record-view-v1.json`. The aggregation templates are already on | ||||||
| `strict_date_optional_time`. If third party packages are implementing their own | ||||||
| statistics they have to update the format too, otherwise the validation would | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| fail. To migrate you can do it offline, by deleting the index and recreating the | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| index (with invenio commands), which adds the new templates to opensearch, | ||||||
| otherwise remove the old template and add the new one by hand. | ||||||
|
Comment on lines
+451
to
+453
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should provide concrete steps here: or (I haven't tested those obviously, but if you provide the exact ones I can try them out too). |
||||||
|
|
||||||
| #### fs | ||||||
|
|
||||||
| Since `pkg-resources` has been deprecated and removed from pypi and the | ||||||
| dependency `fs` has not been updated anymore we decided to reimplement the | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| interface in `invenio-files-rest`. | ||||||
|
|
||||||
| ### fixed warnings | ||||||
|
|
||||||
| - DeprecationWarning: es_clear fixture is deprecated, use es_search instead. | ||||||
| - DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead | ||||||
| - DeprecationWarning: The 'record_to_index' function is no longer expected to return a tuple | ||||||
| - DeprecationWarning: get_user method is deprecated, user get_user_by_email/get_user_by_id | ||||||
| - LegacyAPIWarning: The Query.get() method is considered legacy | ||||||
| - PytestCollectionWarning: cannot collect test class 'TestNAME' because it has a __init__ constructor | ||||||
| - FutureWarning: Truth-testing of elements was a source of confusion | ||||||
| - SyntaxWarning: "\d" is an invalid escape sequence | ||||||
| - ChangedInMarshmallow4Warning: 'Field' should not be instantiated. Use 'fields.Raw' or another field subclass instead. | ||||||
| - RemovedInMarshmallow4Warning: The 'default' argument to fields is deprecated. Use 'dump_default' instead. | ||||||
| - RemovedInMarshmallow4Warning: The 'missing' argument to fields is deprecated. Use 'load_default' instead. | ||||||
| - Warning: JSONSCHEMAS_HOST is set to localhost | ||||||
| - PendingDeprecationWarning: Schema().dump().data and Schema().dump().errors as well as Schema().load().data and Schema().loads().dataattributes are deprecated in marshmallow v3.x. | ||||||
| - DeprecationWarning: refresolver | ||||||
| - SAWarning: his declarative base already contains a class with the same class name and module name | ||||||
| - PytestMockWarning: Mocks returned by pytest-mock do not need to be used as context managers. | ||||||
|
Comment on lines
+461
to
+478
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also internal for us and I wouldn't include it in those public notes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my thinking was, that those things are also interesting for people who will add customization to their instance but are not that deep into the development of InvenioRDM that they are grepping throw every CHANGE.md file of every package. |
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
| ### Optional changes | ||||||
|
|
||||||
| #### Deprecations | ||||||
|
|
@@ -417,4 +501,4 @@ Backend and frontend functionality has been extended to cover related identifier | |||||
| The new feature of allowing replies to comments available in all requests introduces a new config variable `REQUESTS_COMMENT_PREVIEW_LIMIT`, limiting the number of retrieved indexed documents when comments have many replies. | ||||||
| ##### Locking/Unlocking a request's conversation | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This section (and honestly most of those here) should not be under |
||||||
|
|
||||||
| The new feature of allowing locking/unlocking a request's conversation is controlled via a feature flag config variable `REQUESTS_LOCKING_ENABLED`. | ||||||
| The new feature of allowing locking/unlocking a request's conversation is controlled via a feature flag config variable `REQUESTS_LOCKING_ENABLED`. | ||||||
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.
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.
Can you please review this?
#920