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

Add [ QA | ARCHIVER | REPORT ] #546

Merged
merged 14 commits into from
Sep 21, 2022
Merged

Add [ QA | ARCHIVER | REPORT ] #546

merged 14 commits into from
Sep 21, 2022

Conversation

- No need for python2-secrets
- No need to pin pysaml2 (also it's included in saml2auth setup.py... so pip should be happy https://github.com/keitaroinc/ckanext-saml2auth/blob/main/setup.py#L82
- Add in dependencies for qa/report/archiver
@nickumia-reisys nickumia-reisys changed the title Add qa/archiver/report Add [ QA | ARCHIVER | REPORT ] Sep 16, 2022
@nickumia-reisys nickumia-reisys marked this pull request as ready for review September 20, 2022 16:12
@nickumia-reisys nickumia-reisys self-assigned this Sep 20, 2022
@nickumia-reisys nickumia-reisys temporarily deployed to development September 20, 2022 16:36 Inactive
@nickumia-reisys nickumia-reisys temporarily deployed to development September 20, 2022 16:39 Inactive
redis was spelled wrong and ckan might need the celery ini config
@nickumia-reisys nickumia-reisys temporarily deployed to development September 20, 2022 17:26 Inactive
@nickumia-reisys nickumia-reisys requested a review from a team September 21, 2022 14:00
Copy link
Contributor

@jbrown-xentity jbrown-xentity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and could be merged as is. Thanks @nickumia-reisys !

Comment on lines +9 to +11
-e git+https://github.com/gsa/ckanext-qa.git@datagov#egg=ckanext-qa
-e git+https://github.com/ckan/ckanext-archiver.git@master#egg=ckanext-archiver
-e git+https://github.com/ckan/ckanext-report.git@master#egg=ckanext-report
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure, do these need to be installed in editable mode? Did we try without? Either way, could we make a comment (either not tested, or with a note about errors when installing without -e?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not in editable mode, the custom templates are missing and can't be accessed. So yeah, we need it in editable mode. Also, the cli commands don't work if not editable.

(I did try it without editable initially)

Comment on lines +27 to +28
# GSA FIX: e.output is bytes upstream
if "OperationalError" in str(e.output):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we push this fix upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. I think this is a PY2 vs PY3 issue that upstream is still allowing support for PY2 😕

@nickumia-reisys nickumia-reisys merged commit 91b4f47 into main Sep 21, 2022
@nickumia-reisys nickumia-reisys deleted the openness-yay branch September 21, 2022 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants