Set container URLs for tests running in GitHub Actions #1921
Conversation
acdha
force-pushed
the
github-actions-set-test-search-engine-urls
branch
3 times, most recently
from
cbf86fc to
9264f3d
Compare
The old code assumed the non-standard port which the test runner has used for a long, long time to avoid conflicts with other Solr instances on the same host. Since that’s a) ancient history and b) moot in an isolated CI environment, we’ll drop it rather than remapping.
acdha
force-pushed
the
github-actions-set-test-search-engine-urls
branch
from
c97a347 to
3fd8418
Compare
|
@cclauss Some progress but this hit a harder point: GitHub Actions won't let you customize the entry point for a service container which means that we can't easily create the core using the |
Unfortunately, without the ability to customize entrypoints (https://github.com/orgs/community/discussions/26688) this can't use the standard upstream approach unless we want to build a custom container image just for tests: https://solr.apache.org/guide/solr/latest/deployment-guide/solr-in-docker.html#custom-set-up-scripts
acdha
force-pushed
the
github-actions-set-test-search-engine-urls
branch
from
80c06cf to
5d40cff
Compare
.github/workflows/test.yml
Outdated
| run: curl -fs --retry 30 --retry-all-errors --retry-delay 2 http://localhost:8983/solr/admin/cores?action=CREATE&name=collection1&instanceDir=/opt/solr/server/solr/mycores/collection1&config=conf/solrconfig.xml&schema=conf/schema.xml | ||
| - uses: actions/checkout@v3 | ||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v4 |
There was a problem hiding this comment.
Can we use Docker compose to do some magic here:
| status_url = "http://localhost:9001/solr/collection1/admin/ping" | ||
| status_url = "http://%s:%s/solr/collection1/admin/ping" % ( | ||
| os.environ.get("SOLR_HOST", "localhost"), | ||
| os.environ.get("SOLR_PORT", "9001"), |
There was a problem hiding this comment.
Shouldn't we default to the default Solr port?
| os.environ.get("SOLR_PORT", "9001"), | |
| os.environ.get("SOLR_PORT", "8983"), |
There was a problem hiding this comment.
We should - just need to update that in every spot but the old port choices date back to when people were running the test daemons locally without Docker
Co-authored-by: Christian Clauss <cclauss@me.com>
Co-authored-by: Christian Clauss <cclauss@me.com>
|
Please rebase to pick up the |
|
Please rebase to resolve git conflicts. |
|
@acdha Given that the GitHub Actions pass, what is the status of this PR? |
|
@cclauss In your previous comment, did you mean that this PR could be closed now in your opinion? |
I was asking @acdha that question. |
See #1917