-
Notifications
You must be signed in to change notification settings - Fork 7
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
deps: update #168
deps: update #168
Conversation
jrcastro2
commented
Jun 28, 2024
•
edited
Loading
edited
- Add SSPN tab
- closes Bootstrap migration cds-rdm#142
* Add SSPN tab
setup.cfg
Outdated
; python-ldap>=3.4.0,<3.5.0 | ||
invenio-app-rdm[opensearch2]==13.0.0b0.dev4 | ||
invenio-logging[sentry_sdk]>=2.0 | ||
cds-rdm @ git+https://github.com/CERNDocumentServer/cds-rdm#egg=cds-rdm-1.0.0&subdirectory=site |
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.
installing only site folder as dependency (cds-rdm is not installable on it's own since no setup.cfg is present). We might consider adding it maybe
|
||
import os | ||
|
||
from datetime import datetime, timedelta |
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.
this is a copy of our invenio.cfg
Do you maybe have better ideas on how to tackle this problem of duplication?
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.
Let's discuss IRL to be sure we understand well the needs.
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.
Maybe missing to update GitHub Actions and/or tests?
cds_migrator_kit/handlers.py
Outdated
""" | ||
logger = logging.getLogger(f"{rectype}s_logger") | ||
cli_logger.error( | ||
"#RECID: #{0} - {1} MARC FIELD: *{2}*, input value: {3}, -> {4}, ".format( |
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.
General comment for the future: @jrcastro2 will add structured JSON logging to invenio-jobs
, and it would be nice to start using it a bit everywhere when we have want to log long running jobs output, so that we have similar fields/structure.
|
||
import os | ||
|
||
from datetime import datetime, timedelta |
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.
Let's discuss IRL to be sure we understand well the needs.
] | ||
|
||
|
||
class CDSUserEntry(UserEntry): |
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 think we should remove all the users part?
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 left it for now, because I have a vague thought we had implemented it there for a reason initially. Of course I don't remember the reason 🫠
under the terms of the MIT License; see LICENSE file for more details. | ||
#} | ||
|
||
{%- extends config.CDS_MIGRATOR_KIT_BASE_TEMPLATE %} |
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.
Will we have a page per collection? Isn't it always the same template?
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.
this part I need to work on still, but good point, @jrcastro2 do you remember why was this template added?
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.
Right, we still need to get this done. I just set it up this way initially, but if we rename the template to "rdm" and update the rectype in the logger, that should do it I think.
5a13b1a
to
f6a3efd
Compare
* remove the duplicated and outdated transformation code * remove books migration code
* remove redundant files