esmf-org / esmf-profiler

ESMF Profiler application converts binary traces into a web based, dynamic report.
0 stars 2 forks source link

Some files not copied when re-using the same output directory #117

Closed rsdunlapiv closed 2 years ago

rsdunlapiv commented 2 years ago

Describe the bug When re-using the same output directory for the profile output, the updated data/*.json files are not copied in. The issue is that they already exist, and the function used to copy the directly does not overwrite the files.

Specifically, shutil.copytree() used below does not overwrite files.

def _copy_path(src, dst, symlinks=False, ignore=None):
    """Safe copytree replacement"""
    for item in os.listdir(src):
        s = os.path.join(src, item)
        d = os.path.join(dst, item)
        try:
            if os.path.isdir(s):
                shutil.copytree(s, d, symlinks, ignore)
            else:
                shutil.copy2(s, d)
        except FileExistsError as _:  # We don't get this flag until Python 3.8
            continue

See https://github.com/esmf-org/esmf-profiler/blob/74c8039aea890ba74784452be91e0b2d688abbfe/src/esmf_profiler/main.py#L25

To Reproduce Run the esmf-profile twice in a row with the same "-o /path/to/output" option. The second time, the data/*.json files are not overwritten.

Expected behavior All output should be replaced every time you run the profiler, even if reusing an output directory. Probably, shutil.copytree() should be replaced with a function that overwrites existing directories.

ryanlong1004 commented 2 years ago

@rsdunlapiv ran on Ubuntu 20.04; was not able to replicate.

Changing the output name writes over the site.json, re-running the same command changes the timestamp (and file timestamp) in the same file.

I used shutil.copytree with the ignore to prevent .json files in the build directory from being copied over with the template. I don't use this function when generating or writing the .json files, they are the first to be written to the output directory.

image

rsdunlapiv commented 2 years ago

@ryanlong1004 try running this twice in a row

esmf-profiler -t tests/fixtures/test-traces/atm-ocn-concurrent -n slug -o ./slug -p git@github.com:esmf-org/app-profiles.git

On my machine:

(venv) rocky@rocky-Latitude-E5570:~/esmfdev/esmf-profiler.nov19$ esmf-profiler -t tests/fixtures/test-traces/atm-ocn-concurrent -n slug -o ./slug -p git@github.com:esmf-org/app-profiles.git
esmf_profiler.main : Processing trace: tests/fixtures/test-traces/atm-ocn-concurrent
esmf_profiler.main : Generating 1 profiles
esmf_profiler.main : Succesfully pushed ./slug to git@github.com:esmf-org/app-profiles.git
(venv) rocky@rocky-Latitude-E5570:~/esmfdev/esmf-profiler.nov19$ esmf-profiler -t tests/fixtures/test-traces/atm-ocn-concurrent -n slug -o ./slug -p git@github.com:esmf-org/app-profiles.git
esmf_profiler.main : Processing trace: tests/fixtures/test-traces/atm-ocn-concurrent
esmf_profiler.main : Generating 1 profiles
esmf_profiler.git : On branch main
Your branch is up to date with 'origin/main'.

nothing to commit, working tree clean

esmf_profiler.main : Succesfully pushed ./slug to git@github.com:esmf-org/app-profiles.git
(venv) rocky@rocky-Latitude-E5570:~/esmfdev/esmf-profiler.nov19$ 

Note the output from git: nothing to commit, working tree clean

This means that the new site.json file (with updated timestamp) was not copied in -- there should always be something to commit.

Does this behave the same on your system? Or is specific to mine?

ryanlong1004 commented 2 years ago

@rsdunlapiv After testing, this is from the identical commit messages.

We can add a hash to every commit, guaranteeing each push; is this the direction to go?

rsdunlapiv commented 2 years ago

I'm still not clear on this one. The content of a commit message is not used by git to determine if the working tree is clean, i.e. if there is something to commit.

I recommend adding some debug logging when you are copying in the files, and I think you will see that some files are not copied over to the cloned repo directory because they already exist.

On Mon, Nov 22, 2021, 7:29 PM Ryan Long @.***> wrote:

@rsdunlapiv https://github.com/rsdunlapiv After testing, this is from the identical commit messages.

We can add a hash to every commit, guaranteeing each push; is this the direction to go?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/esmf-org/esmf-profiler/issues/117#issuecomment-976108373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYR3FSAT7ZDSN5F2YJCPALUNL37ZANCNFSM5ILAWE5A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.