New Celery app, remove kubernetes / flask, update tests to iRODS 4.3.3 #267

alanking · 2024-09-11T22:36:28Z

Addresses #211
Addresses #262
Addresses #269
Addresses #272
Addresses #274

Some notes on this PR...

Main test suite is passing.
I have not tested S3 yet.
The existing Celery application and many of its constellation of files have been removed.

I don't think we need any new tests because the goal is for the new implementation to be identical in behavior to the old implementation besides having new Celery application and task names.

Also, I realize this is a big change, so here's a few areas where eyeballs are most needed (IMO):

New file and directory names
New Celery task names (look for @app.task)
Any other re-organization efforts that seem like they would be good to include here

Will leave in draft til I can test with S3.

trel

please run black if you hadn't already.

those imports all look pretty organized, so maybe you already did it...

irods_capability_automated_ingest/irods/filesystem.py

irods_capability_automated_ingest/custom_event_handler.py

alanking · 2024-09-12T02:04:04Z

Ah yes. I'll run black just before we # this because it's going to make rebasing very tricky later should I need to squash things.

alanking · 2024-09-12T13:39:48Z

Nevermind, I found a way to thread the needle. Had to run the formatter on the test suite before new changes came in and things work out pretty cleanly. Force-pushed because still in draft. Will test S3 shortly...

trel · 2024-09-12T13:50:43Z

nice.

alanking · 2024-09-12T17:37:35Z

Had to make a tweak and then was able to successfully sync an S3 bucket with Minio in the demo Docker Compose project. So, I guess this is ready for review.

FifthPotato

First set of review comments, mostly on one file. I'll post more on the rest later, since this is a pretty hefty PR.

irods_capability_automated_ingest/irods/irods_utils.py

alanking

I'll contemplate making the suggested changes in this PR. The primary goal of this PR is to restructure the project under a new Celery app and making the tasks a little more straightforward, so the existing implementation was retained as much as possible. That said, the suggested changes are valid concerns which should be fixed.

irods_capability_automated_ingest/irods/irods_utils.py

alanking · 2024-09-16T18:48:06Z

Caught an issue in the latest changes... Tests are passing again.

FifthPotato

Alright, that should be the rest of them. Just a couple more small things!

irods_capability_automated_ingest/irods/s3_bucket.py

irods_capability_automated_ingest/tasks/filesystem_sync.py

alanking

Thanks, @FifthPotato

irods_capability_automated_ingest/tasks/filesystem_sync.py

irods_capability_automated_ingest/irods/s3_bucket.py

alanking · 2024-09-17T18:02:13Z

Tests are passing with latest changes

alanking · 2024-09-19T17:36:47Z

Rebased and squashed. Didn't mean to push, though. Sorry about that, reviewers...

trel · 2024-09-19T17:49:39Z

very shady.

FifthPotato

Looks good to me!

korydraughn

I noticed a few TODO comments in here. Let's get some issue numbers attached to them and get this merged if everything's working.

irods_capability_automated_ingest/tasks/filesystem_sync.py

korydraughn · 2024-09-25T19:27:27Z

We got an approval from @FifthPotato.

Do you feel this is ready for squashing/merging? Is there anything else left to do for this PR?

alanking · 2024-09-25T19:32:37Z

I think this is ready. The goal was to keep the behavior unchanged as much as possible and the tests are passing, so I think adding anything else in this PR is unnecessary if we're all good with the existing changes.

korydraughn · 2024-09-25T19:36:52Z

Sounds good to me.

Please squash to taste.

alanking · 2024-09-25T19:42:42Z

Squashed

korydraughn

Pound and merge it.

The kubernetes directory and flask app are not part of the main Automated Ingest framework or the sync script. This introduces unnecessary dependencies and clutters the codebase. These have been removed.

Updates the ingest-test Docker Compose project to install iRODS 4.3.3 on the iRODS catalog service provider service. Also runs black formatter on the test suite Python file.

This commit creates an alternative implementation of the ingest Celery app. The most relevant changes to the project structure are described below. The Celery application is now housed in celery.py which allows the Celery application run by the workers to refer simply to the name of the Python module irods_capability_automated_ingest instead of requiring irods_capability_automated_ingest.sync_task. The Celery application includes the tasks defined in the tasks subdirectory. The tasks subdirectory includes a file for syncing a filesystem and a file for syncing an S3 bucket. There is currently a lot of duplicate code between the two, but separating these is an important step for adding more types of storage for syncing. In order to remove circular dependencies, an irods subdirectory has also been added to provide the functionality for interactions with PRC. The Celery tasks call into these functions just like they always had, but, as they always have, storage-specific implementations are needed in order to implement things like reading data and performing parallel transfers.

This commit updates README instructions, tests, examples and the irods_sync script to use the new Celery application and tasks.

This commit removes the historical implementation of the Automated Ingest framework, including the Celery application, tasks, and sync machinery. A new Celery application has been created in its place.

This commit makes some changes to irods_session to prevent empty strings in environment variables and other potential issues caused by invalid inputs.

alanking · 2024-09-25T19:48:20Z

#'d, mergin

trel reviewed Sep 12, 2024

View reviewed changes

irods_capability_automated_ingest/irods/filesystem.py Outdated Show resolved Hide resolved

irods_capability_automated_ingest/custom_event_handler.py Outdated Show resolved Hide resolved

alanking force-pushed the new-celery-app branch from a079445 to 9fd243f Compare September 12, 2024 13:39

alanking force-pushed the new-celery-app branch from 9fd243f to 8770418 Compare September 12, 2024 17:36

alanking marked this pull request as ready for review September 12, 2024 17:37

This was referenced Sep 12, 2024

error adding metadata to data object with grave character in the path #100

Closed

S3 sync tasks hang when destination sub-collections do not exist #124

Closed

korydraughn requested a review from FifthPotato September 12, 2024 18:29

FifthPotato reviewed Sep 12, 2024

View reviewed changes

alanking commented Sep 13, 2024

View reviewed changes

FifthPotato reviewed Sep 17, 2024

View reviewed changes

alanking mentioned this pull request Sep 17, 2024

Investigate parallel_upload_from_S3 chunking logic #273

Open

alanking commented Sep 17, 2024

View reviewed changes

alanking force-pushed the new-celery-app branch from fe0446c to bc73f32 Compare September 19, 2024 17:35

korydraughn requested a review from FifthPotato September 19, 2024 18:45

FifthPotato approved these changes Sep 25, 2024

View reviewed changes

korydraughn reviewed Sep 25, 2024

View reviewed changes

irods_capability_automated_ingest/tasks/filesystem_sync.py Outdated Show resolved Hide resolved

alanking force-pushed the new-celery-app branch from bc73f32 to 7954e19 Compare September 25, 2024 19:15

alanking force-pushed the new-celery-app branch from 7954e19 to f25caa8 Compare September 25, 2024 19:42

korydraughn approved these changes Sep 25, 2024

View reviewed changes

alanking added 8 commits September 25, 2024 15:47

[irods#262] Remove kubernetes directory and flask app

0ed3474

The kubernetes directory and flask app are not part of the main Automated Ingest framework or the sync script. This introduces unnecessary dependencies and clutters the codebase. These have been removed.

[irods#180] Testing: Update to iRODS 4.3.3, run black formatter

e8a745f

Updates the ingest-test Docker Compose project to install iRODS 4.3.3 on the iRODS catalog service provider service. Also runs black formatter on the test suite Python file.

[irods#211] Use new Celery application and tasks

480f801

This commit updates README instructions, tests, examples and the irods_sync script to use the new Celery application and tasks.

[irods#211] Remove 'icai' Celery application

1804ac4

This commit removes the historical implementation of the Automated Ingest framework, including the Celery application, tasks, and sync machinery. A new Celery application has been created in its place.

[irods#269] Improvements to irods_session

33afb33

This commit makes some changes to irods_session to prevent empty strings in environment variables and other potential issues caused by invalid inputs.

[irods#274] Refactor parts of sync_data_from_file

24e8d5a

[irods#272] Fixes for exclude_file_type

8786afe

alanking force-pushed the new-celery-app branch from f25caa8 to 8786afe Compare September 25, 2024 19:48

alanking merged commit bd88378 into irods:main Sep 25, 2024

alanking deleted the new-celery-app branch September 25, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Celery app, remove kubernetes / flask, update tests to iRODS 4.3.3 #267

New Celery app, remove kubernetes / flask, update tests to iRODS 4.3.3 #267

alanking commented Sep 11, 2024 •

edited

Loading

trel left a comment

alanking commented Sep 12, 2024

alanking commented Sep 12, 2024

trel commented Sep 12, 2024

alanking commented Sep 12, 2024

FifthPotato left a comment

alanking left a comment

alanking commented Sep 16, 2024

FifthPotato left a comment

alanking left a comment

alanking commented Sep 17, 2024

alanking commented Sep 19, 2024

trel commented Sep 19, 2024

FifthPotato left a comment

korydraughn left a comment

korydraughn commented Sep 25, 2024

alanking commented Sep 25, 2024

korydraughn commented Sep 25, 2024

alanking commented Sep 25, 2024

korydraughn left a comment

alanking commented Sep 25, 2024

New Celery app, remove kubernetes / flask, update tests to iRODS 4.3.3 #267

New Celery app, remove kubernetes / flask, update tests to iRODS 4.3.3 #267

Conversation

alanking commented Sep 11, 2024 • edited Loading

trel left a comment

Choose a reason for hiding this comment

alanking commented Sep 12, 2024

alanking commented Sep 12, 2024

trel commented Sep 12, 2024

alanking commented Sep 12, 2024

FifthPotato left a comment

Choose a reason for hiding this comment

alanking left a comment

Choose a reason for hiding this comment

alanking commented Sep 16, 2024

FifthPotato left a comment

Choose a reason for hiding this comment

alanking left a comment

Choose a reason for hiding this comment

alanking commented Sep 17, 2024

alanking commented Sep 19, 2024

trel commented Sep 19, 2024

FifthPotato left a comment

Choose a reason for hiding this comment

korydraughn left a comment

Choose a reason for hiding this comment

korydraughn commented Sep 25, 2024

alanking commented Sep 25, 2024

korydraughn commented Sep 25, 2024

alanking commented Sep 25, 2024

korydraughn left a comment

Choose a reason for hiding this comment

alanking commented Sep 25, 2024

alanking commented Sep 11, 2024 •

edited

Loading