Update integrated test baseline storage #14

cssherman · 2024-03-15T17:36:16Z

cssherman · 2024-03-15T17:53:20Z

@TotoGaz - any thoughts on this approach for managing test baselines? Reading the docs, these google-cloud python API calls can accept credentials from the user's environment, which is consistent with how you are currently uploading test artefacts on the CI.

Also, we'll need to work out access details for manual updates, testing, etc.

cssherman · 2024-03-15T17:54:42Z

I'm currently relying on reading/writing to yaml file to set the latest version of the baselines. An example of one is in the linked PR, here: https://github.com/GEOS-DEV/GEOS/pull/3044/files#diff-6daa8e7f57a28d97ee7ea9fe3b51a6cbbcb58a59633488c885e402ded0445c2f

cssherman · 2024-03-15T17:56:58Z

geos_ats_package/geos_ats/baseline_io.py

+        client = storage.Client()
+        bucket = client.bucket( bucket_name )
+        blob = bucket.blob( blob_name )
+        blob.download_to_filename( archive_name )


These methods are currently specific to GCP, but looking at the Azure docs, their API is very similar: https://learn.microsoft.com/en-us/azure/storage/blobs/storage-blob-upload-python

BTW, there also exists a direct link you can use as a std url.

My thoughts were that we could benefit from some of the error checking / progress messages baked into this package, rather than rolling our own. Also, I'm noticing that we should set use_auth_w_custom_endpoint=False (allows an anonymous user, see https://cloud.google.com/python/docs/reference/storage/latest/google.cloud.storage.client)

That's fine. We should test this is in proxy constrained environments.
I think that what can be a critical feature is how complicated (or not) it is to define a proxy (and doc on how to) with our downloading package.

TotoGaz

I think we can continue this way.
Maybe do we need some time to validate?

geos_ats_package/geos_ats/baseline_io.py

geos_ats_package/geos_ats/command_line_parsers.py

geos_ats_package/geos_ats/baseline_io.py

TotoGaz · 2024-03-20T04:02:17Z

geos_ats_package/geos_ats/command_line_parsers.py

        exit_flag = True

+    if exit_flag:
+        for option_type, details in zip( [ 'action', 'check' ], [ action_options, check_options ] ):


Suggested change

for option_type, details in zip( [ 'action', 'check' ], [ action_options, check_options ] ):

for option_type, details in ('action', action_options), ('check', check_options ):

does that work? I am not so sure. I think you need to make it a list of tuples.

>>> for i, j in (0, 1), (2, 3): ... print(i, j) ... 0 1 2 3

I guess there's some auto conversion to an Iterable on the fly.

@CusiniM I was surprised by this behavior myself when @TotoGaz suggested it

geos_ats_package/setup.cfg

CusiniM · 2024-04-04T00:38:53Z

The code looks fine to me but we should chat about the usage because something is unclear to me. For example, I don't fully understand the difference between deleting and updating. I guess the delete is in case someone wants to download a newer version?

And I have also missed where we are storing the hash of the baselines in the GEOS repo...And are we expecting users to upload baselines manually or is this done automatically by the CI?if done automatically how are we triggering it upon validation?

TotoGaz · 2024-04-04T04:13:22Z

geos_ats_package/geos_ats/baseline_io.py

+    path.parent.mkdir( parents=True, exist_ok=True )
+
+    r = requests.get( url, stream=True, allow_redirects=True, headers=headers )
+    if r.status_code != 200:


Suggested change

if r.status_code != 200:

if not r.ok:

? I'm not sure.

was this supposed to be changed?

Forgot about that one. It may need some more testing, so let's save it for the next upgrade

TotoGaz · 2024-04-04T20:42:39Z

geos_ats_package/geos_ats/baseline_io.py

+    try:
+        r.raw.read = partial( r.raw.read, decode_content=True )
+        with tqdm.wrapattr( r.raw, "read", total=file_size, desc=desc ) as r_raw:
+            with path.open( "wb" ) as f:
+                shutil.copyfileobj( r_raw, f )
+
+    except:
+        with path.open( "wb" ) as f:
+            for chunk in r.iter_content( chunk_size=128 ):
+                f.write( chunk )


I do not understand the need of this try/expect. Could you elaborate?

seems to be a fallback method in case that tqdm library fails to download the file.

More or less. On some systems, the default method can run into issues, and this is a fall-back that seems to be quite reliable.

TotoGaz · 2024-04-04T20:48:11Z

geos_ats_package/geos_ats/baseline_io.py

+    if os.path.isdir( baseline_path ):
+        logger.info( f'Target baseline directory already exists: {baseline_path}' )


Disputable, but maybe

if not os.path.isdir( baseline_path ): os.makedirs( os.path.dirname( baseline_path ), exist_ok=True ) else: logger.info( f'Target baseline directory already exists: {baseline_path}' ) ...

would remove the one line else at the very end (which comes as a surprise)?

TotoGaz · 2024-04-04T20:52:00Z

geos_ats_package/geos_ats/baseline_io.py

+    if os.path.isfile( archive_name ):
+        # Unpack new baselines
+        logger.info( f'Unpacking baselines: {archive_name}' )
+        try:
+            shutil.unpack_archive( archive_name, baseline_path, format='gztar' )
+            logger.info( 'Finished fetching baselines!' )
+        except Exception as e:
+            logger.error( str( e ) )
+            raise Exception( f'Failed to unpack baselines: {archive_name}' )
+
+    else:
+        logger.error( str( e ) )
+        raise Exception( f'Could not find baseline files to unpack: expected={archive_name}' )


Suggested change

if os.path.isfile( archive_name ):

# Unpack new baselines

logger.info( f'Unpacking baselines: {archive_name}' )

try:

shutil.unpack_archive( archive_name, baseline_path, format='gztar' )

logger.info( 'Finished fetching baselines!' )

except Exception as e:

logger.error( str( e ) )

raise Exception( f'Failed to unpack baselines: {archive_name}' )

else:

logger.error( str( e ) )

raise Exception( f'Could not find baseline files to unpack: expected={archive_name}' )

if not os.path.isfile( archive_name ):

logger.error( str( e ) )

raise Exception( f'Could not find baseline files to unpack: expected={archive_name}' )

# Unpack new baselines

try:

logger.info( f'Unpacking baselines: {archive_name}' )

shutil.unpack_archive( archive_name, baseline_path, format='gztar' )

logger.info( 'Finished fetching baselines!' )

except Exception as e:

logger.error( str( e ) )

raise Exception( f'Failed to unpack baselines: {archive_name}' )

again, disputable

cssherman added 2 commits March 14, 2024 17:09

Adding python scripts to manage baseline io

ff14ade

Wiring in new baseline management tools

d8b0e14

cssherman mentioned this pull request Mar 15, 2024

Update integrated test baseline storage GEOS-DEV/GEOS#3044

Merged

Fixing typo in geos_ats parser

f3d2860

cssherman requested a review from TotoGaz March 15, 2024 17:49

cssherman commented Mar 15, 2024

View reviewed changes

cssherman requested review from CusiniM and rrsettgast March 15, 2024 17:58

Fixing package import bug

93fcf9c

cssherman added the type: feature label Mar 15, 2024

cssherman self-assigned this Mar 15, 2024

TotoGaz approved these changes Mar 15, 2024

View reviewed changes

Updating geos_ats input args, various baseline method updates

dccdc28

TotoGaz reviewed Mar 20, 2024

View reviewed changes

cssherman added 9 commits March 21, 2024 13:58

Updating geos ats command line parsing

6e4ce12

Adding yaml path to ats environment setup

bb68813

Splitting baseline archive packing, upload

d9b4c11

Fixing bug in baseline packing

97f063e

Adding https baseline fetch option

80ae85c

Adding options to work with baseline cache files

0bacdce

Fixing baseline archive names

7325ae0

Fixing baseline archive structure, copying log files

0b0591c

Fixing blob download name

a7facad

CusiniM approved these changes Apr 4, 2024

View reviewed changes

TotoGaz reviewed Apr 4, 2024

View reviewed changes

TotoGaz approved these changes Apr 4, 2024

View reviewed changes

Handling empty directories for baseline management

8a17acc

cssherman added 12 commits April 10, 2024 17:06

Fixing log copying bug

6c1a49d

Removing test messages from geos_ats

b35d7f8

Adding simple log check script

49af05e

Updating log checker for geos ats

fead5c9

Fixing log checker script

2ceda4f

Fixing log check script

683f82b

Attempting to use an anonymous gcp client for baseline fetch

b7da058

Fixing log check, allowing baselines to be packed to various folders

070fc12

Fixing geos ats blob name

cbed575

Adding whitelist for geos ats log check script

37bcbfb

Adding yaml input option to geos_ats log checker

f752546

Using relative file paths for geos_ats html logs

a4b40a7

untereiner mentioned this pull request Apr 16, 2024

mesh_doctor installation via pip #18

Open

cssherman added 15 commits April 16, 2024 15:17

Updating ats html table format

63d9624

Updating html report style

f154225

Removing auto page refresh from geos ats report

107bebc

Updating html report style

3e74652

Updating html report style

0f0e48e

Adding additional assets to html report

8669589

Fixing report label

64a5f4c

Fixing lightbox settings

a33c9d5

Grouping lightbox captions

a2d4194

Adding baseline history file

ab0bc6b

Fixing the baseline log path

97f7d41

Separating logs from archives by default

8d7c3dc

Fixing parsing of geos ats options

cad1f59

Skipping baseline management for some test actions

218af63

Adding an additional prerequisite to geos_ats

b82a757

CusiniM approved these changes May 3, 2024

View reviewed changes

CusiniM merged commit 60511cf into main May 7, 2024
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update integrated test baseline storage #14

Update integrated test baseline storage #14

cssherman commented Mar 15, 2024 •

edited

Loading

cssherman commented Mar 15, 2024

cssherman commented Mar 15, 2024

cssherman Mar 15, 2024

TotoGaz Mar 16, 2024

cssherman Mar 18, 2024

TotoGaz Mar 18, 2024

TotoGaz left a comment

TotoGaz Mar 20, 2024

CusiniM Apr 4, 2024

TotoGaz Apr 4, 2024

cssherman Apr 4, 2024

CusiniM commented Apr 4, 2024 •

edited

Loading

TotoGaz Apr 4, 2024 •

edited

Loading

CusiniM May 3, 2024

cssherman May 3, 2024

TotoGaz Apr 4, 2024

CusiniM May 3, 2024

cssherman May 3, 2024

TotoGaz Apr 4, 2024

TotoGaz Apr 4, 2024

	for option_type, details in zip( [ 'action', 'check' ], [ action_options, check_options ] ):
	for option_type, details in ('action', action_options), ('check', check_options ):

		if os.path.isdir( baseline_path ):
		logger.info( f'Target baseline directory already exists: {baseline_path}' )

Update integrated test baseline storage #14

Update integrated test baseline storage #14

Conversation

cssherman commented Mar 15, 2024 • edited Loading

cssherman commented Mar 15, 2024

cssherman commented Mar 15, 2024

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

TotoGaz 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

CusiniM commented Apr 4, 2024 • edited Loading

TotoGaz Apr 4, 2024 • 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

cssherman commented Mar 15, 2024 •

edited

Loading

CusiniM commented Apr 4, 2024 •

edited

Loading

TotoGaz Apr 4, 2024 •

edited

Loading