[BUGFIX] Allows GE to work when installed in a zip file (PEP 273). Fixes issue #3772

[joseignaciorc]

About this PR

This PR closes the issue #3772.

Changes proposed in this pull request:

• html_site_store.py has been modified. Now when GE is installed in a zip file, the static files are extracted (i.e. unziped) before they are used, so that the context.build_data_docs() function works as expected instead of failing with a FileNotFoundError or NotADirectoryError
• test-pep273-compatibility.yml has been created. A GA (Github Action) that verifies that the context.build_data_docs() function works when GE is installed in a directory and also in a zip. This verification is done by trying to run the simple_build_data_docs.py (which also created in this PR)

How these changes were tested:

• First, I created a test that proves that, with the current code in the develop branch, context.build_data_docs() fails in zips: https://github.com/joseignaciorc/great_expectations/actions/runs/1542371481
• Then, I fixed the bug and now context.build_data_docs() works in all cases: https://github.com/joseignaciorc/great_expectations/actions/runs/1542425347

IMPORTANT:
The tests on test-pep273-compatibility are set to run on every push to the repo. The test is lightweight, but I understand that a Github Action may or may not the best way to test this change, so I am open to discuss changes in the GA and also in the code.

Definition of Done

Please delete options that are not relevant.

• My code follows the Great Expectations style guide
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added unit tests where applicable and made sure that new and existing tests are passing.
• I have run any local integration tests and made sure that nothing is broken.

[netlify]

👷 Deploy request for niobium-lead-7998 pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: fc64df1

[cdkini]

Hey @joseignaciorc thanks so much for contributing!

Let me confer internally to determine how best to navigate running this in CI/CD. I'll bring it up during our next engineering meeting and get back to you!

[cdkini]

Minor comments but the core logic looks great! Still conferring internally about best practices around testing.

[cdkini]

Since this logic is very specific to zip-files, could we extract the logic into a separate method? Maybe something like this:

def _handle_zipped_static_assets(self) -> bool:
       # If `static_assets_source_absdir` contains the string ".zip/" (unix) or ".zip\" (win)
        # then we check if Great Expectations has been installed inside a zip file (see PEP 273)
        # If so, we need to extract all the static files and run this function again
        static_assets_source_absdir = os.path.abspath(static_assets_source_dir)
        zip_re = re.match(
            f"(.+[.]zip){re.escape(os.sep)}(.+)",
            static_assets_source_absdir,
            flags=re.IGNORECASE,
        )

        if zip_re:
            zip_filename = zip_re.groups()[0]  # e.g.: /home/joe/libs/my_python_libs.zip
            path_in_zip = zip_re.groups()[1]  # great_expectations/render/view/static
            unzip_destination = tempfile.mkdtemp()

            if is_zipfile(zip_filename):
                with ZipFile(zip_filename) as zipfile:
                    static_files_to_extract = [
                        file
                        for file in zipfile.namelist()
                        if file.startswith(path_in_zip)
                    ]
                    zipfile.extractall(unzip_destination, static_files_to_extract)
                self.copy_static_assets(unzip_destination)
                return True
       return False

That way, we can isolate the zip-specific code and simply invoke it in copy_static_assets

unzipped = self._handle_zipped_static_files()
if unzipped:
    return

Thoughts?

[cdkini]

Could we add a quick blurb that explains the purpose of this test?

"""
What does this test and why?

Ensures compatibility with PEP-273: https://www.python.org/dev/peps/pep-0273/
Covers the use-case where GE is installed in a zip file and build_data_docs() is called.
Any other relevant details you may deem relevant!
"""

[cdkini]

I just added your test to our integration test matrix; I think that'll probably be the way to go here just so we don't test on each change!

[joseignaciorc]

I have added a new commit with the changes you commented, @cdkini

Extracting the zip is now done in its own function (_unzip_assets()) and the test in simple_build_data_docs.py is now documented