-
Notifications
You must be signed in to change notification settings - Fork 60
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
Starter site integration #287
Starter site integration #287
Conversation
... so the URL is specified appropriately by default, and the user can be specified when the migrations run.
... something of a typo that was being propagated.
@@ -164,7 +164,7 @@ run-islandora-migrations: | |||
#docker-compose exec -T drupal with-contenv bash -lc "for_all_sites import_islandora_migrations" | |||
# this line can be reverted when https://github.com/Islandora-Devops/isle-buildkit/blob/fae704f065435438828c568def2a0cc926cc4b6b/drupal/rootfs/etc/islandora/utilities.sh#L557 | |||
# has been updated to match | |||
docker-compose exec -T drupal with-contenv bash -lc 'drush -l $(SITE) migrate:import islandora_defaults_tags,islandora_tags' | |||
docker-compose exec -T drupal with-contenv bash -lc 'drush -l $(SITE) migrate:import $(MIGRATE_IMPORT_USER_OPTION) islandora_defaults_tags,islandora_tags' |
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.
If the MIGRATE_IMPORT_USER_OPTION
variable is not otherwise specified, $(MIGRATE_IMPORT_USER_OPTION)
should just give the empty string, which should continue to run this as it was... but can now specify the variable (as something like MIGRATE_IMPORT_USER_OPTION=--userid=1
(as is being done under the starter-finalize
make target)) in order to have it perform the migrate:import
command as a particular user, of whom we can assure the fedoraAdmin
role is present.
@@ -117,6 +117,7 @@ CODE_SERVER_PORT=8443 | |||
############################################################################### | |||
|
|||
DOMAIN=islandora.traefik.me | |||
SITE=https://${DOMAIN} |
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.
Given this is provided to many(/most?) drush
commands, it makes sense to have it default to something that should be valid for looking up the JWT key instead of the default
.
- drupal-sites-data:/var/www/drupal/web/sites/default/files | ||
- solr-data:/opt/solr/server/solr | ||
environment: | ||
DRUPAL_DEFAULT_INSTALL_EXISTING_CONFIG: ${INSTALL_EXISTING_CONFIG} |
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 (and starter_dev
's) are just copypasta from the docker-compose.local.yml
... wherein it seems this value isn't actually used? As in: grep'ing here in isle-dc
, just finding it being specified, and grep'ing in the isle-buildkit
, it seems to pass the variable in the environment; however, it does not appear to be used in any kind of site:install
wrapper... indeed, the site:install
(/si
) is in our Makefile
(where we would have to pass the --existing-config
option)... so it's... kind of implied that this shouldn't do anything?
#docker-compose exec -T drupal with-contenv bash -lc 'drush migrate:rollback islandora_defaults_tags,islandora_tags' |
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.
Copypasta from the local
target. Could be dropped if we want?
docker-compose exec -T drupal with-contenv bash -lc "drush -l $(SITE) user:role:add fedoraadmin admin" | ||
MIGRATE_IMPORT_USER_OPTION=--userid=1 $(MAKE) hydrate | ||
docker-compose exec -T drupal with-contenv bash -lc 'drush -l $(SITE) migrate:import --userid=1 islandora_fits_tags' |
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.
Just for reference, this islandora_fits_tags
migration is being introduced in roblib/islandora_fits#14 , and has been included in islandora/islandora-starter-site
as a patch: https://github.com/Islandora/islandora-starter-site/blob/b954409a0fb47dbbef0a415f25cf66821ce023bb/composer.json#L97-L101
## Make a local site with codebase directory bind mounted, using cloned starter site. | ||
starter_dev: QUOTED_CURDIR = "$(CURDIR)" | ||
starter_dev: generate-secrets | ||
$(MAKE) starter-init ENVIRONMENT=starter_dev | ||
if [ -z "$$(ls -A $(QUOTED_CURDIR)/codebase)" ]; then \ | ||
docker container run --rm -v $(CURDIR)/codebase:/home/root $(REPOSITORY)/nginx:$(TAG) with-contenv bash -lc 'git clone -b main https://github.com/Islandora/islandora-starter-site /home/root;'; \ | ||
fi | ||
$(MAKE) set-files-owner SRC=$(CURDIR)/codebase ENVIRONMENT=starter_dev | ||
docker-compose up -d --remove-orphans | ||
docker-compose exec -T drupal with-contenv bash -lc 'composer install' | ||
$(MAKE) starter-finalize ENVIRONMENT=starter_dev |
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.
Yeah, unsure if the starter_dev
target makes sense for isle-dc
, as the instance comes up with a "dirty" config, due to having set URLs with hostnames and the like.
... ideally, all properties that are not portable between environments could be moved to use Drupal's "State API", or if that's not feasible, to be injected with something like config_override
, or more generally via Drupal config override system, which should keep changes to such values out of the exported configs... but such is outside the scope of the present work.
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.
Discussed this comment with Will and we also came to the consensus that it is outside of scope.
@@ -339,17 +339,17 @@ demo: generate-secrets | |||
## Make a local site with codebase directory bind mounted, modeled after sandbox.islandora.ca | |||
local: QUOTED_CURDIR = "$(CURDIR)" | |||
local: generate-secrets | |||
$(MAKE) download-default-certs ENVIROMENT=local | |||
$(MAKE) download-default-certs ENVIRONMENT=local |
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.
Did a %s/ENVIROMENT/ENVIRONMENT/g
... grep'd first for ENVIROMENT
, and couldn't find any other references trying to make use of this typo'd variant (between isle-dc
and isle-buildkit
), so changed 'em all over.
Tech call: Does this require other PRs to go in first? If so, should this be changed to Draft? |
@DonRichards: No, none that I'm aware of, though I have been away from things for over a week, due to Hurricane Fiona. Which specific PRs do you think there might be, upon which this would be dependent? |
I don't have any specific. Just was curious if there was any need for other PRs to go in first. |
Thought the Github editor would automatically include one (when resolving the merge conflict), but apparently, it does not.
Analog to Islandora-Devops/islandora-playbook#226, in the context of isle-dc...
Pretty much copypasta from "local"...