Draft:Add provenance functionality #1993

kinow · 2024-12-13T01:24:00Z

In GitLab by @apuiggro on Oct 28, 2024, 12:18

…ed of archiving (Now testing)

* add CI and dependabot gh actions * disable linting temporarily

* Autosubmit config version compatibility fix ( rebasing ) Reverting a conflict in the rebase to the other option II ( tests are only failing in the CI/CD ) Reverting a conflict in the rebase to the other option updated typing added a missing return Changed err_code function to return the err_code Rebase fix Squashed zombies/orphan and database fixes. PIPELINE Fix doc test Fix pipeline Added comments and typing (II ) Added comments and typing Test pipeline ( now with output) Test pipeline Update as version Fix tests, added pytest-xdist !475 vertical regression test [Single job] Test working, retrials working Changes to stat, db and retrials (WIP) Added database test, fixed some issues reduced time.sleeps Mock exists() so it works in the pipeline Fix events queue being reset in a platform failure clean import added more tests Changed the timeout removed debug message Added tests for signals cleanup code Added cleanup signal changes to signals and waitpid A new orphan/zombies code approach WIP: Looking good, some more fixes neccesaries and ready to be pushed WIP: Zombies/orphan processors logs fix * Pipeline failing, debug (I) * Fix Stat_0 file being deleted when it shouldn't * Fixed issues with the db and logs not being recovered in different run scenearios * Update VERSION * added process_jobs_to_submit fixes runtime issue * Added few todos that requires to change critical stuff * Fix runtime issue, recovery now shows a better output * Improved package test * Improved package test * Closing file descriptors on reconnect. * Fixes an issue with logs with this sequence of commands: `create + run + create + recovery + run` * Intregation test added, fixed few bugs * Fixed DB test * Now the db test also checks the stat and job_names in the filesystem * feedback * Changed test name, Added generic functions in the test to expand it with other run options * Added few todos * disabled the check exit code as it is not working on the pipeline and not important right now * Adding more chunks to the wrapper success * More detailed test * Add CI and dependabot GitHub actions (#2021) * add CI and dependabot gh actions * disable linting temporarily (cherry picked from commit 8a1b2df) * Improved test output in case of failure * more info * more info * Test adding +x * added chmod after sending * chmod added --------- Co-authored-by: dbeltran <daniel.beltran@bsc.es> Co-authored-by: Luiggi Tenorio <luiggibit@gmail.com> --------- Co-authored-by: dbeltran <daniel.beltran@bsc.es> Co-authored-by: Luiggi Tenorio <luiggibit@gmail.com> --------- Co-authored-by: dbeltran <daniel.beltran@bsc.es> Co-authored-by: Luiggi Tenorio <luiggibit@gmail.com>

* Added notify_on * added update_stat_file() (wrong rebase) --------- Co-authored-by: dbeltran <daniel.beltran@bsc.es>

* Fix issue with the wrapper attribute * Fix issue with the wrapper attribute

…a vertical wrapper (#2038)

* Fixes wrapper warning message at the end * Added typing suggestion

kinow

Sorry the delay in reviewing this, @apuiggro . Left some comments & suggestions. Can you rebase again, please.

kinow · 2024-12-27T11:23:19Z

.gitignore

@@ -26,3 +26,6 @@ dockerfiles/id_rsa*
 dockerfiles/authorized_keys

 venv/
+
+#metadata
+metadata/


Is this ignore necessary to build the project? Or was it used for some local test? if it's not part of the AS build then it's better to not include it.

Just saw you have a metadata/data/job_data_a000.sql file in this change. I think you need to configure AS to write metadata in another directory. Can you remove that file if that's not needed for this PR?

kinow · 2024-12-27T11:23:40Z

autosubmit/provenance/rocrate.py

@@ -555,8 +556,13 @@ def create_rocrate_archive(
        patch = json.loads(rocrate_json['PATCH'])
        for jsonld_node in patch['@graph']:
            crate.add_or_update_jsonld(jsonld_node)
-
+    


Unnecessary extra spaces ☝️

kinow · 2024-12-27T11:29:17Z

autosubmit/provenance/rocrate.py

+    #zip is defined in this function so all the (modified) code should be moved here. Still, I'm exploring other posibilites
+    #to query the las modified time within this function (Not very used to the code still...)
+    date = time.strftime("%Y%m%d%H%M%S", time.localtime(os.path.getmtime(path)))
+    crate.write_zip(Path(path, f"{expid}-{date}.zip"))


I think it'd be safer to go with the local time. The time the command was run.

Similar to how it's done in other commands like profiler, report, etc.:

datetime.datetime.today().strftime('%Y%m%d-%H%M%S')

Okey! Should I leave it this way then?

date = datetime.datetime.today().strftime('%Y%m%d-%H%M%S')

kinow · 2024-12-27T11:30:25Z

autosubmit/autosubmit.py

@@ -649,7 +655,7 @@ def parse_args():
            subparser.add_argument('-v', '--update_version', action='store_true',
                                   default=False, help='Update experiment version')
            subparser.add_argument('--rocrate', action='store_true', default=False,
-                                   help='Unarchive an RO-Crate file')
+                                  help='Unarchive an RO-Crate file')


🙆‍♂️ thanks!

kinow · 2024-12-27T11:31:10Z

autosubmit/autosubmit.py

@@ -2266,7 +2274,7 @@ def run_experiment(expid, notransitive=False, start_time=None, start_after=None,
                        job_list.update_list(as_conf, submitter=submitter)
                        job_list.save()
                        # Submit jobs that are ready to run
-                        #Log.debug(f"FD submit: {fd_show.fd_table_status_str()}")
+                        #Log.debug(f"FD submit: {fd_show.fd_table_status_str()}")run_experiment


Was it a test? Maybe remove the run_experiment (I think this might be used by dani for tesing -- although this can probably be refactored/removed in the future).

Mmm... I believe I haven't added that (at least intentionally). I'll remove it :).

kinow · 2024-12-27T11:40:09Z

autosubmit/autosubmit.py

+        :type expid: str
+        :param rocrate: flag to enable RO-Crate
+        :type rocrate: bool
+        """""


Too many quotes here. It's just three or one 🙂

kinow · 2024-12-27T11:53:17Z

autosubmit/autosubmit.py

+                f"Error creating RO-Crate ZIP file: {str(e)}", 7012)
+        else:
+           raise AutosubmitCritical(
+                    "Can not create RO-Crate ZIP file. Argument '--rocrate' required", 7012) 


I think we can invert the logic here, so that we don't create the Path unnecessarily, and leave the validation/constraint logic first, followed by the main logic. Something like,

@staticmethod def provenance(expid, rocrate=False): """ :param expid: experiment identifier :type expid: str :param rocrate: flag to enable RO-Crate :type rocrate: bool """ if not rocrate: msg = "Can not create RO-Crate ZIP file. Argument '--rocrate' required" raise AutosubmitCritical(msg, 7012) aslogs_folder = Path( BasicConfig.LOCAL_ROOT_DIR, expid, BasicConfig.LOCAL_TMP_DIR, BasicConfig.LOCAL_ASLOG_DIR ) try: Autosubmit.rocrate(expid, Path(aslogs_folder)) Log.info('RO-Crate ZIP file created!') except Exception as e: raise AutosubmitCritical(f"Error creating RO-Crate ZIP file: {str(e)}", 7012)

kinow · 2024-12-27T11:56:25Z

test/unit/test_provenance.py

+            Autosubmit.provenance("expid123", rocrate=False)
+
+        assert "Can not create RO-Crate ZIP file. Argument '--rocrate' required" in str(excinfo)
+        mock_rocrate.assert_not_called() 


Needs to have 2 spaces between each top-level function. My IDE warns when this happens, but linters like pep8/ruff/etc.

kinow · 2024-12-27T12:02:31Z

test/unit/test_provenance.py

+from autosubmit.autosubmit import Autosubmit
+from log.log import AutosubmitCritical
+import os
+from unittest.mock import patch


If you are using VS Code or PyCharm they should have the option to automatically sort the imports (we will enable this in CICD later). In PyCharm there is the option when you right-click a file, "Optimize Imports".

kinow · 2024-12-27T12:08:10Z

test/unit/test_provenance.py

@@ -0,0 +1,59 @@
+import pytest


Tests look great! The rocrate function is already covered, so good strategy to only cover the parts missing 👍

…ed of archiving (Now testing)

codecov-commenter · 2025-01-03T09:22:11Z

Codecov Report

Attention: Patch coverage is 62.06897% with 132 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@0e6032c). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
autosubmit/platforms/platform.py	38.18%	34 Missing ⚠️
autosubmit/autosubmit.py	64.15%	14 Missing and 5 partials ⚠️
autosubmit/platforms/wrappers/wrapper_factory.py	45.45%	18 Missing ⚠️
autosubmit/platforms/slurmplatform.py	25.00%	15 Missing ⚠️
autosubmit/job/job_list.py	54.83%	7 Missing and 7 partials ⚠️
autosubmit/job/job.py	85.50%	8 Missing and 2 partials ⚠️
autosubmit/job/job_packager.py	64.28%	7 Missing and 3 partials ⚠️
autosubmit/platforms/paramiko_platform.py	58.33%	2 Missing and 3 partials ⚠️
autosubmit/platforms/ecplatform.py	50.00%	2 Missing ⚠️
autosubmit/platforms/sgeplatform.py	0.00%	2 Missing ⚠️
... and 3 more

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1993   +/-   ##
=========================================
  Coverage          ?   48.92%           
=========================================
  Files             ?       72           
  Lines             ?    17564           
  Branches          ?     3418           
=========================================
  Hits              ?     8594           
  Misses            ?     8161           
  Partials          ?      809

Flag	Coverage Δ
fast-tests	`48.92% <62.06%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

apuiggro · 2025-01-03T10:52:41Z

Hi @kinow,

Rebase finished! I believe all the issues are solved. Let me know if there is any other problem.

Thankss

apuiggro and others added 20 commits October 2, 2024 10:24

Started with the modifications of the rocrate generation

6495bb5

Added provenance functionality to generate the rocrate without the ne…

2e1af32

…ed of archiving (Now testing)

Unit test draft for provenance function

f902753

Unit test finished, now the code need to be reviewed

b2180f1

Fixed archive/unarchive definitions and unit test for provenance

7f9845a

Fixed some aspects of the code

d337f27

Run provenance feature

ff94e76

Removed run/provenance feature to another branch

4c6c0d0

Provenance generation when running

7c760a5

Updated branch with main

7af6f98

Gitignore update

58d6d27

Add CI and dependabot GitHub actions (#2021)

8a1b2df

* add CI and dependabot gh actions * disable linting temporarily

hotfix: add master to action trigger

9a3275e

add codeql advanced usage

c2589a6

re-added notify_on (#2020)

156b6e7

* Added notify_on * added update_stat_file() (wrong rebase) --------- Co-authored-by: dbeltran <daniel.beltran@bsc.es>

Fix issue with the is_wrapper attribute (#2033)

e8877c9

* Fix issue with the wrapper attribute * Fix issue with the wrapper attribute

Changed the Log so it display what retrials it downloaded when it is …

7b3e226

…a vertical wrapper (#2038)

Fixes wrapper warning message at the end closes #2035 (#2037)

451a25c

* Fixes wrapper warning message at the end * Added typing suggestion

updated timeout for pipeline

3706cb9

kinow commented Dec 27, 2024

View reviewed changes

kinow mentioned this pull request Dec 27, 2024

Draft: Run provenance #2017

Open

LuiggiTenorioK and others added 8 commits December 27, 2024 13:20

update test_rocrate.py to use ruamel.yaml

0ad5a34

Started with the modifications of the rocrate generation

893f80d

Added provenance functionality to generate the rocrate without the ne…

eae16e9

…ed of archiving (Now testing)

Unit test draft for provenance function

1ee4743

Unit test finished, now the code need to be reviewed

d373034

Fixed archive/unarchive definitions and unit test for provenance

3b00c7d

Fixed some aspects of the code

557cebc

Run provenance feature

29fbe8d

apuiggro added 3 commits January 2, 2025 10:34

Gitignore update

7aeaedb

Rebase

91eab4b

Changes for PR

523846a

apuiggro added 3 commits January 3, 2025 10:51

several changes to PR

d7a19aa

Minor changes for PR

064f9e1

Rebase finished

912b404

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft:Add provenance functionality #1993

Draft:Add provenance functionality #1993

kinow commented Dec 13, 2024

kinow left a comment •

edited

Loading

kinow Dec 27, 2024

kinow Dec 27, 2024

kinow Dec 27, 2024

kinow Dec 27, 2024

apuiggro Jan 2, 2025

kinow Jan 2, 2025

kinow Dec 27, 2024

kinow Dec 27, 2024

apuiggro Jan 2, 2025

kinow Dec 27, 2024

kinow Dec 27, 2024

kinow Dec 27, 2024

kinow Dec 27, 2024

kinow Dec 27, 2024

codecov-commenter commented Jan 3, 2025 •

edited

Loading

apuiggro commented Jan 3, 2025

Draft:Add provenance functionality #1993

Are you sure you want to change the base?

Draft:Add provenance functionality #1993

Conversation

kinow commented Dec 13, 2024

kinow left a comment • edited Loading

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

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

codecov-commenter commented Jan 3, 2025 • edited Loading

Codecov Report

apuiggro commented Jan 3, 2025

kinow left a comment •

edited

Loading

codecov-commenter commented Jan 3, 2025 •

edited

Loading