Accept multiple download ids and expire all queued option #78#79

Merged

patrick-austin merged 25 commits into36_queuingfrom

78_multiple_download_ids

Apr 2, 2025

patrick-austin commented Apr 1, 2025

Closes #78

patrick-austin and others added 18 commits

January 2, 2025 14:09


          Add warning/justification to getEntity javadoc #35

9d6d004


          Merge pull request #39 from ral-facilities/35_test_improvements

13f725f

Use timeout, actual ICAT ids and Long.equals in UserResourceTest #35


          Bump ch.qos.logback:logback-classic from 1.2.12 to 1.2.13

82bce1f

Bumps [ch.qos.logback:logback-classic](https://github.com/qos-ch/logback) from 1.2.12 to 1.2.13.
- [Commits](qos-ch/logback@v_1.2.12...v_1.2.13)

---
updated-dependencies:
- dependency-name: ch.qos.logback:logback-classic
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>


          Merge pull request #31 from ral-facilities/dependabot/maven/ch.qos.lo…

9a6b8ee

…gback-logback-classic-1.2.13

Bump ch.qos.logback:logback-classic from 1.2.12 to 1.2.13


          Bump ch.qos.logback:logback-classic in /provision/skeleton_plugin

ddfa983

Bumps [ch.qos.logback:logback-classic](https://github.com/qos-ch/logback) from 1.1.7 to 1.2.13.
- [Commits](qos-ch/logback@v_1.1.7...v_1.2.13)

---
updated-dependencies:
- dependency-name: ch.qos.logback:logback-classic
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>


          Merge pull request #30 from ral-facilities/dependabot/maven/provision…

bc105dc

…/skeleton_plugin/ch.qos.logback-logback-classic-1.2.13

Bump ch.qos.logback:logback-classic from 1.1.7 to 1.2.13 in /provision/skeleton_plugin


          Replace mysql:mysql-connector-java with com.mysql:mysql-connector-j

a47f901


          Merge pull request #59 from ral-facilities/mysql_connector_j_9.1.0

ac39254

Replace mysql:mysql-connector-java with com.mysql:mysql-connector-j


          Enable dependabot for GHA #64

b3f02a8


          Merge pull request #65 from ral-facilities/64_enable_dependabot_gha

dec2bad

Enable dependabot for GHA #64


          Bump actions/cache from 2 to 4

ac10bde

Bumps [actions/cache](https://github.com/actions/cache) from 2 to 4.
- [Release notes](https://github.com/actions/cache/releases)
- [Commits](actions/cache@v2...v4)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>


          Merge pull request #66 from ral-facilities/dependabot/github_actions/…

f744a6b

…actions/cache-4

Bump actions/cache from 2 to 4


          Bump actions/setup-java from 1 to 4

cfce982

Bumps [actions/setup-java](https://github.com/actions/setup-java) from 1 to 4.
- [Release notes](https://github.com/actions/setup-java/releases)
- [Commits](actions/setup-java@v1...v4)

---
updated-dependencies:
- dependency-name: actions/setup-java
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>


          Merge pull request #67 from ral-facilities/dependabot/github_actions/…

5dfdadb

…actions/setup-java-4


          Bump nick-invision/assert-action from 1 to 2

78f56b5

Bumps [nick-invision/assert-action](https://github.com/nick-invision/assert-action) from 1 to 2.
- [Release notes](https://github.com/nick-invision/assert-action/releases)
- [Changelog](https://github.com/nick-fields/assert-action/blob/main/.releaserc.js)
- [Commits](nick-fields/assert-action@v1...v2)

---
updated-dependencies:
- dependency-name: nick-invision/assert-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>


          Merge pull request #68 from ral-facilities/dependabot/github_actions/…

d4e3ffe

…nick-invision/assert-action-2


          Add new POST /admin/download/{id}/prepare endpoint #74

f654848


          Accept multiple download ids and expire all queued option #78

2c2a5ad

patrick-austin requested a review from kevinphippsstfc

April 1, 2025 16:40

kevinphippsstfc requested changes

View reviewed changes

tools/datagateway_admin Outdated

-              		"sessionId": session_id,
-              		"queryOffset": "where download.id = " + download_id
-              	}, verify=verifySsl).text)
+              	download_ids = input("Enter download id(s): ")

kevinphippsstfc Apr 2, 2025

As per the other PR, we should clarify what the separator is eg:
"Enter download id(s) - space separated if multiple:"

tools/datagateway_admin Outdated

-              	else:
-              		for datafile_location in datafile_locations:
-              			print(datafile_location)
+              	download_ids = input("Enter download id(s): ")

kevinphippsstfc Apr 2, 2025

As per the other PR, we should clarify what the separator is eg:
"Enter download id(s) - space separated if multiple:"

tools/datagateway_admin Outdated



		def expire_download():
		download_ids = input("Enter download id(s): ")

kevinphippsstfc Apr 2, 2025

As per the other PR, we should clarify what the separator is eg:
"Enter download id(s) - space separated if multiple:"

tools/datagateway_admin Outdated

               def start_queued_download():
-              	download_id = input("Enter download id: ")
-              	prepare_download(download_id=download_id)
+              	download_ids = input("Enter download id(s): ")

kevinphippsstfc Apr 2, 2025

As per the other PR, we should clarify what the separator is eg:
"Enter download id(s) - space separated if multiple:"

tools/datagateway_admin Outdated

-              	}
-              	url = topcat_url + "/admin/download/" + download_id +  "/status"
-              	requests.put(url=url, data=data, verify=verifySsl)
+              	download_ids = input("Enter download id(s): ")

kevinphippsstfc Apr 2, 2025

As per the other PR, we should clarify what the separator is eg:
"Enter download id(s) - space separated if multiple:"

tools/datagateway_admin Outdated

		@@ -387,7 +406,8 @@ while True:
		print(" * 10: Start a queued download.")

kevinphippsstfc Apr 2, 2025

This should probably now be "Start queued download(s)." with this pattern being applied to all the other options that now accept multiple IDs.

tools/datagateway_admin Outdated

               	print(" * 11: Start all queued downloads.")
               	print(" * 12: Re-queue expired download.")
-              	print(" * 13: Exit")
+              	print(" * 13: Expire all queued download.")

kevinphippsstfc Apr 2, 2025

download needs to be downloads

patrick-austin and others added 4 commits

April 2, 2025 08:20


          Pluralise prompts for prepare_data in admin script #74

00df092


          Merge pull request #77 from ral-facilities/74_prepare_data_endpoint

a71e439

Add new POST /admin/download/{id}/prepare endpoint #74


          Merge branch 'master' into 78_multiple_download_ids

613ca15


          Refactor download id splitting into shared function #78

b7d3ce1

Author

patrick-austin commented Apr 2, 2025

Merged master to get the changes from #77 so I could refactor the id splitting to one place. This also means a lot of older commits showing on the diff here.

patrick-austin requested a review from kevinphippsstfc

April 2, 2025 09:23

patrick-austin added 3 commits

April 2, 2025 09:57


          installTrustManager BeforeClass StatusCheckTest

62e0c27


          Remove datasets from dummy download, assert queued downloads reach RE…

16b3015

…STORING


          Merge branch '36_queuing' into 78_multiple_download_ids

6b5525f

Author

patrick-austin commented Apr 2, 2025

TestHelpers.createDummyDownload was creating DownloadItems for Datasets with ids hardcoded to 10, 11, 12. This is fine if any of the following are true:

Your Database is populated from scratch with ids starting at 1 before every test execution (this is the case for the CI)
You mock the IdsClient so that you never actually send a request to it (this is the case for the existing StatusCheckTest tests)
You use a "DummySessionId" as this will always cause an error, meaning we never reach the RESTORING state and assert for PREPARING instead (this was the case until the changes from Use functional credentials for prepareData queue calls to IDS #72 )

So after #72 the CI can get to RESTORING, but because of the first point my local tests still ended up in PREPARING as the IDS 404s when trying to look for Dataset 10 (since it doesn't exist in my local DB with that id). In general, I do not think we should hardcode these magic id numbers, so I tried removing that entire section from the Dummy Download function and all tests passed for me locally (i.e. nothing we were using the Dummy downloads for actually needed there to be DownloadItems set).

kevinphippsstfc approved these changes

View reviewed changes

patrick-austin merged commit 35c5c61 into 36_queuing

1 check passed

patrick-austin deleted the 78_multiple_download_ids branch

April 2, 2025 12:38

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet