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

fix: Fix commandline argument passing to runBlazegraph.sh #364

Merged
merged 1 commit into from Sep 30, 2022

Conversation

kwisatz
Copy link
Contributor

@kwisatz kwisatz commented Sep 9, 2022

This commit fixes two issues in the container scripts that prevented users from passing any options to blazegraph (as documented here: https://www.mediawiki.org/wiki/Wikidata_Query_Service/User_Manual#Configurable_properties)

This commit changes two things:

  • Rather than overwrite $BLAZEGRAPH_OPTS in entrypoint.sh, it extends it using $EXTRA_BLAZEGRAPH_OPTS
  • It modifies the root directoy runBlazegraph.sh to no longer drop all arguments passed to the docker command property

An example of setting EXTRA_BLAZEGRAPH_OPTS in the docker-compose files would be for example:

wdqs:
    image: wdqs:latest
    restart: unless-stopped
[…]
    environment:
      - EXTRA_BLAZEGRAPH_OPTS=-Dhttps.proxyHost=192.168.48.1 -Dhttps.proxyPort=8888 -Dhttp.proxyHost=192.168.48.1 -Dhttp.proxyPort=8888
      - WIKIBASE_HOST=foobar.local
      - WDQS_HOST=wdqs.svc
      - WDQS_PORT=9999

@addshore
Copy link
Contributor

addshore commented Sep 9, 2022

Great, I can make the changes and get this merged

@addshore
Copy link
Contributor

addshore commented Sep 9, 2022

Scratch that, I can't alter this branch :)

@kwisatz
Copy link
Contributor Author

kwisatz commented Sep 9, 2022 via email

@kwisatz
Copy link
Contributor Author

kwisatz commented Sep 13, 2022

I am unsure as to whether the build failures are related to my changes? Could someone with more experience with the pipeline please advise?

@addshore
Copy link
Contributor

I am unsure as to whether the build failures are related to my changes? Could someone with more experience with the pipeline please advise?

Re running them now

addshore added a commit that referenced this pull request Sep 22, 2022
This would help debug issues such as
#364
@addshore
Copy link
Contributor

if you could rebase on #371 the extra log output should help us see what is happening with wdqs

@digitaalwerktuig
Copy link

Just checking to inform about the status of this PR. Anything @kwisatz can do to advance this?

addshore added a commit that referenced this pull request Sep 27, 2022
* Always output docker logs in CI

This would help debug issues such as
#364

* fix path

* Actually add file

* Fix SC2086
@kwisatz
Copy link
Contributor Author

kwisatz commented Sep 28, 2022

@addshore just rebased it

@addshore
Copy link
Contributor

From the more verbose output in CI now I see this

Container: test_wdqs-updater_1
/entrypoint.sh: line 16: BLAZEGRAPH_OPTS: unbound variable
/entrypoint.sh: line 16: BLAZEGRAPH_OPTS: unbound variable
/entrypoint.sh: line 16: BLAZEGRAPH_OPTS: unbound variable

Container: test_wdqs_1

@kwisatz
Copy link
Contributor Author

kwisatz commented Sep 28, 2022

Not sure what happened here, I must have messed up the PR somehow, this is not what I should have in HEAD. I'll look into it.

Yup… my mistake.

@addshore
Copy link
Contributor

I believe you need to provide a default var in the image, or the var in /test/docker-compose.upgrade.wdqs.yml.

In the image probably makes the most sense if the default should be empty and the entrypoint expects it

@kwisatz
Copy link
Contributor Author

kwisatz commented Sep 28, 2022

@addshore which build-step did those appear in earlier?

@addshore
Copy link
Contributor

A note about the env var should also be added to Docker/build/WDQS/README.md

This commit fixes two issues in the container scripts that prevented
users from passing any options to blazegraph (as document here:
https://www.mediawiki.org/wiki/Wikidata_Query_Service/User_Manual#Configurable_properties)

This commit changes two things:
 - Rather than overwrite $BLAZEGRAPH_OPTS in entrypoint.sh, it extends
   it using $BLAZEGRAPH_EXTRA_OPTS
 - It modifies the root directoy runBlazegraph.sh to no longer drop all
   arguments passed to the docker command property
@addshore addshore merged commit 6b76b25 into wmde:main Sep 30, 2022
hirokiu added a commit to hirokiu/wikibase-release-pipeline that referenced this pull request Oct 6, 2022
* commit '42951edfd4edd68f112077ba05078934e8a6b8f6': (62 commits)
  Poke interwiki name and source name
  Use assert.deepStrictEqual
  $ is webdriver, not jqeury /o\
  mariadb:10.9
  Use 1.37.6
  Try mw-content-text for edit page content
  Use assert package consistently
  Fix test: entityschema.js
  Use assert.strictEqual in tests
  debug logging of mw in tests
  mwlogs, continue on error & cleaner ls output
  mwlog, more ls output, and no /
  Poke the logging again...
  Try outputting mw logs again
  fix: Fix commandline argument passing to runBlazegraph.sh (wmde#364)
  Add wgWBClientSettings['itemAndPropertySourceName']
  Revert "Revert "Revert "Output MediaWiki logs in CI too"""
  Revert "More general path for mwlog output"
  More general path for mwlog output
  Revert "Revert "Output MediaWiki logs in CI too""
  ...
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.

None yet

3 participants