Bacpop 208 Migration for visualise update #9

absternator · 2025-02-27T15:30:24Z

The following is a migration to update projects before the updates made to combine network + microreact call into 1 visualise call. as per https://github.com/bacpop/beebop_py.

There are 4 main data updates made.

update redis keys with name migration. These are all updated to be called visualise from now on.
add pruned graphml if not exists. makes copy called pruned which is used by frontend
rename microreact folders to visualise including all prefix names. eg. microreact_9 --> visualise_9
move network files from network folder to corresponding visualuse_X folder. The cytoscape.csv file is copied into all visualise_X folders in project folder. The graphml files are copied as per respective clustern number. eg. /network/network_component_9.graphml --> /visualise_9/visualise_9_component_9.graphml.

Note there will still be issues with very old projects before v9 upgrade where cluster microreact_X, where X was randomly arbritary. As now it is the cluster number. But these networks are not going to work regardless of PR changes

This has been run on dev and can be tested there.
Testing:
Head to https://beebop-dev.dide.ic.ac.uk/ and ensure your old projects load and work.

absternator · 2025-02-27T15:30:52Z

TOOD: merge in beebop and beebop_py and revert this branch to main and then merge this PR in... Then can run on prod

EmmaLRussell

Looks pretty tidy to me!

EmmaLRussell · 2025-03-03T09:57:56Z

migrations/20250227_viz_move/migration.py

+    # Get all valid output folders
+    all_output_folders = [
+        os.path.join(base_folder, item)
+        for item in os.listdir(base_folder)
+        if os.path.isdir(os.path.join(base_folder, item))
+    ]
+
+    # Filter for folders with network folder (i.e old output folders to update)
+    output_folders = [
+        folder
+        for folder in all_output_folders
+        if os.path.exists(os.path.join(folder, "network"))
+    ]


Could just do these in one loop I guess and save a few lines, though you would have one unpythonically long line...

Suggested change

# Get all valid output folders

all_output_folders = [

os.path.join(base_folder, item)

for item in os.listdir(base_folder)

if os.path.isdir(os.path.join(base_folder, item))

]

# Filter for folders with network folder (i.e old output folders to update)

output_folders = [

folder

for folder in all_output_folders

if os.path.exists(os.path.join(folder, "network"))

]

# Get all valid output folders

output_folders = [

os.path.join(base_folder, item)

for item in os.listdir(base_folder)

if os.path.isdir(os.path.join(base_folder, item)) and os.path.exists(os.path.join(base_folder, item "network"))

]

EmmaLRussell · 2025-03-03T10:04:46Z

migrations/20250227_viz_move/migration.py

+    if r.exists("beebop:hash:job:microreact"):
+        microreact_data = r.hgetall("beebop:hash:job:microreact")
+        for field, value in microreact_data.items():
+            r.hset("beebop:hash:job:visualise", field, value)


This isn't actually renaming the key is it? It's leaving the microreact key and values as they are, but just copying them into the visualise key. Which is fair enough since the visualise key may already exist by the time you run the migration, and I'm not sure you'd want to do a RENAME in that case. (Plus it wouldn't be rollback-able.) So maybe just change the comment and log message as they're a little misleading..?

EmmaLRussell · 2025-03-03T10:08:36Z

migrations/20250227_viz_move/cleanup.sh

+# Remove backup_folder inside the container if exists
+if docker exec $CONTAINER_NAME test -d $BACKUP_FOLDER; then
+    docker exec $CONTAINER_NAME rm -rf $BACKUP_FOLDER
+    echo "Successfully restored PopPUNK output from backup."


This looks like the wrong log message!

EmmaLRussell · 2025-03-03T10:09:31Z

migrations/20250227_viz_move/cleanup.sh

+fi
+
+# Remove migration.py inside the container
+docker exec $CONTAINER_NAME rm -f /beebop/storage/migration.py


If this is the point of no return for the migration, perhaps this is also where you should be deleting the old redis keys too? I don't think that's happening otherwise..?

oh yup good idea

EmmaLRussell · 2025-03-03T10:25:14Z

migrations/20250227_viz_move/migration.py

+                    os.path.join(parent_folder, dir_name)
+                ) and dir_name.startswith("visualise_"):
+                    # Rename file
+                    new_fname = file.replace("network", dir_name)


This is just to get a new file name? Were all the old files called "network.csv" and this is rename them to "visualise_CLUSTERNUMBER.csv"?

EmmaLRussell · 2025-03-03T10:28:19Z

migrations/20250227_viz_move/migration.py

+
+                    if not os.path.exists(new_fname_path):
+                        shutil.copyfile(
+                            os.path.join(network_folder, file), new_fname_path


So this is renaming it in the network folder, and then it subsequently it moves it to the visualise folder? Why not just do both of those things in one copyfile?

yeah i realised that later im stupid... il just leave as its working and I don't wanna break it... (as we don't have to maintain this code shouldn't be a biggie)

EmmaLRussell · 2025-03-03T10:30:49Z

migrations/20250227_viz_move/migration.py

+                    logger.info(f"Renamed file: {file} → {new_fname}")
+
+                # Move file to visualise folder
+                if not os.path.exists(os.path.join(visualise_path, new_fname)):


Same question here - why not just rename and move in one go?

as above comment im stupid

…on scripts

…ttps://github.com/bacpop/beebop-deploy into bacpop-208-migration

…ction

…rity

… for clarity" This reverts commit 8e13b6f.

…al requests

…cal requests

…g environment variable for CA bundle

… integration tests

EmmaLRussell

All looks good. Not sure why the changes to the integration test needed..?

EmmaLRussell · 2025-03-06T10:10:42Z

test/test_integration.py

+    # Give services time to fully initialize
+    time.sleep(5)


oh dear! wonder why that's crept in...

yeah i tried so many things(as you can see from commits haha) and this was the only things let It actually consistently pass without have SSL / connection errors

EmmaLRussell · 2025-03-06T10:11:24Z

test/test_integration.py

@@ -27,8 +26,11 @@ def test_start_beebop():
    session = requests.Session()
    session.verify = False
    session.trust_env = False
-    os.environ['CURL_CA_BUNDLE'] = ""


what was this about?

was depricated in requests version 2.28

absternator added 2 commits February 27, 2025 13:53

visualise call migration

929046e

upfate migreations

2941efd

absternator requested a review from EmmaLRussell February 27, 2025 15:31

EmmaLRussell reviewed Mar 3, 2025

View reviewed changes

pr comments resolve

f37e09f

absternator requested a review from EmmaLRussell March 5, 2025 09:09

absternator added 23 commits March 5, 2025 09:15

pipeline use ubuntu 22.04

52b12b5

fix python version

a55d8ba

revert ubuntu + python changed

3a972b1

addd refs logs

e5d95b8

add log

97390d4

refactor: clean up whitespace and improve logging messages in migrati…

ac511b7

…on scripts

remove logs

d44361c

fix: suppress urllib3 SSL warnings in integration test

e95ea86

Merge branches 'bacpop-208-migration' and 'bacpop-208-migration' of h…

c879a54

…ttps://github.com/bacpop/beebop-deploy into bacpop-208-migration

refactor: simplify SSL handling in integration test

812a960

feat: implement comprehensive SSL handling in integration test

3e85b57

refactor: enhance SSL handling in make_secure_request function

3e26eb1

refactor: streamline SSL handling in make_secure_request function

222be00

refactor: improve SSL context setup in make_secure_request function

559faca

refactor: remove redundant SSL context setup in integration test

ac4c7cb

feat: implement comprehensive SSL handling in make_secure_request fun…

4f41ef3

…ction

refactor: remove SSL verification bypass in integration test

d67f478

refactor: simplify SSL context setup in make_secure_request function

3eec712

refactor: rename make_secure_request to make_insecure_request for cla…

8e13b6f

…rity

Revert "refactor: rename make_secure_request to make_insecure_request…

4fe02d1

… for clarity" This reverts commit 8e13b6f.

refactor: enhance SSL handling and add retry logic in integration test

6489c0c

refactor: update integration test to disable SSL verification for loc…

cdf03ba

…al requests

{commit_message}

d1472bf

absternator added 19 commits March 5, 2025 13:37

refactor: reorganize integration tests and add API endpoint verification

28f3d3c

refactor: enhance integration test with SSL handling and retry logic

ebaaa53

refactor: update integration test to use HTTP instead of HTTPS for lo…

8782e80

…cal requests

refactor: update integration test to use HTTP for API requests

0d19f69

refactor: simplify integration test by removing SSL handling and usin…

d57b51b

…g environment variable for CA bundle

refactor: add TLSAdapter for custom SSL handling in integration tests

2c4345e

{commit_message}

93a880b

refactor: specify requests version in requirements.txt

913a437

fix: update requests version to 2.27.1 in requirements.txt

089ca49

{commit_message}

a1df2d7

refactor: remove specific requests version and adjust SSL handling in…

dac6dab

… integration tests

test: add delay in integration test for stable response

57373a2

{commit_message}

3df47e3

{commit_message}

724be87

{commit_message}

d78857a

test: remove unused environment variable setting in integration test

c4f663e

test: remove TLSAdapter from integration test

0621e04

test: clean up imports in integration test

73f4af2

config: update image tags to use 'main' for beebop components

3378aa0

EmmaLRussell approved these changes Mar 6, 2025

View reviewed changes

absternator merged commit 9ce9868 into main Mar 6, 2025
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bacpop 208 Migration for visualise update #9

Bacpop 208 Migration for visualise update #9

absternator commented Feb 27, 2025 •

edited

Loading

absternator commented Feb 27, 2025

EmmaLRussell left a comment

EmmaLRussell Mar 3, 2025

EmmaLRussell Mar 3, 2025

EmmaLRussell Mar 3, 2025

EmmaLRussell Mar 3, 2025

absternator Mar 5, 2025

EmmaLRussell Mar 3, 2025

EmmaLRussell Mar 3, 2025

absternator Mar 5, 2025

EmmaLRussell Mar 3, 2025

absternator Mar 5, 2025

EmmaLRussell left a comment

EmmaLRussell Mar 6, 2025

absternator Mar 6, 2025 •

edited

Loading

EmmaLRussell Mar 6, 2025

absternator Mar 6, 2025

Bacpop 208 Migration for visualise update #9

Bacpop 208 Migration for visualise update #9

Conversation

absternator commented Feb 27, 2025 • edited Loading

absternator commented Feb 27, 2025

EmmaLRussell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmaLRussell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

absternator Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

absternator commented Feb 27, 2025 •

edited

Loading

absternator Mar 6, 2025 •

edited

Loading