Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

New ckanext-grouphierarchy version and personalised license list #31

Merged
merged 5 commits into from
Aug 6, 2023

Conversation

MarijaKnezevic
Copy link
Collaborator

No description provided.

@MarijaKnezevic
Copy link
Collaborator Author

Changes are tested locally with docer/compose and with sddi-ckan-k8s

helm install ckan sddi-ckan/sddi-ckan --set "ckan.image.tag=sddi-base-pr-31-43be262" --set "ckan.image.repository=ghcr.io/tum-gis/ckan-sddi-dev" -n ckan --create-namespace --atomic --wait --timeout 10m --values "https://raw.githubusercontent.com/tum-gis/sddi-ckan-k8s/main/examples/docker-desktop/values-local.yml"

@MarijaKnezevic MarijaKnezevic requested a review from BWibo August 3, 2023 15:17
@@ -159,6 +166,7 @@ for production environments.**
### Known issues

[Unreleased]: https://github.com/tum-gis/ckan-docker/compare/1.1.2...HEAD
Copy link
Member

Choose a reason for hiding this comment

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

Increase version here too, to show commits between latest release and latest commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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


### Fixed

- upstream bugfix from [ckanext-grouphierarchy-sddi](https://github.com/tum-gis/ckanext-grouphierarchy-sddi) extension tum-gis/ckanext-grouphierarchy-sddi#17
Copy link
Member

Choose a reason for hiding this comment

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

Please describe shortly, what was fixed. Just letting people know, that this is an upstream bugfix is not helpful.

@@ -192,6 +192,7 @@ RUN set -ex && \
ckan config-tool "${CKAN_INI}" "scheming.dataset_schemas = ckanext.scheming:ckan_dataset.yaml" && \
ckan config-tool "${CKAN_INI}" "scheming.presets = ckanext.scheming:presets.json ckanext.repeating:presets.json ckanext.composite:presets.json" && \
ckan config-tool "${CKAN_INI}" "scheming.dataset_fallback = false" && \
ckan config-tool "${CKAN_INI}" "licenses_group_url = https://raw.githubusercontent.com/tum-gis/ckanext-grouphierarchy-sddi/main/ckanext/grouphierarchy/licenses_SDDI.json" &&\
Copy link
Member

Choose a reason for hiding this comment

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

  • Missing space after &&
  • Is setting this here really required? If this file is the default, it should be shipped with the extension (which is the case, according to the URL) and set as the default value (local path inside the container) in the extension. The user should only have to take action, if he needs to overwrite this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is an extended license list which is required for SDDI. With CKAN installation it comes default licenses list.
With the URL which is defined in mentioned Dockerfile, I want to overwrite CKAN Default value and use customized list.

This value could be defined as ENV var which I've added in our docker/compose installation draft.

@BWibo BWibo self-requested a review August 6, 2023 09:48
@BWibo BWibo merged commit 3c1e03f into devel Aug 6, 2023
@BWibo BWibo deleted the devel-update branch August 6, 2023 09:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants