Skip to content
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

Merged
merged 12 commits into from
Jul 17, 2024
Merged

deps: update #168

merged 12 commits into from
Jul 17, 2024

Conversation

jrcastro2
Copy link
Contributor

@jrcastro2 jrcastro2 commented Jun 28, 2024

* 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
Copy link
Contributor

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
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor

@ntarocco ntarocco left a 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?

"""
logger = logging.getLogger(f"{rectype}s_logger")
cli_logger.error(
"#RECID: #{0} - {1} MARC FIELD: *{2}*, input value: {3}, -> {4}, ".format(
Copy link
Contributor

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
Copy link
Contributor

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):
Copy link
Contributor

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?

Copy link
Contributor

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 %}
Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor Author

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.

@kpsherva kpsherva force-pushed the update-deps branch 2 times, most recently from 5a13b1a to f6a3efd Compare July 15, 2024 12:40
@kpsherva kpsherva merged commit 26b51b2 into master Jul 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bootstrap migration
3 participants