Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Enhance platform check for stackql installation(macOS) #45

Merged
merged 3 commits into from
Sep 21, 2024

Conversation

derek10cloud
Copy link
Contributor

@derek10cloud derek10cloud commented Sep 18, 2024

This PR enhances the reliability of stackql installation on MacOS + Fix Test Codes by:

1) Modifying the platform check to use startswith('Darwin') instead of strict equality. This ensures compatibility with various Darwin-based OS versions.

When _setup function check the platform, MacOS user's platform is not excatly Darwin,
but it just starts with Darwin, so I changed the condition to check if platform.startswith('Darwin').
image

2) Adding a step to clear any existing files in the unpacked directory before extracting the new stackql binary. This prevents potential conflicts and ensures a clean installation.

[before]

command = 'pkgutil --expand-full {} {}'.format(archive_file_name, unpacked_file_name)
os.system(command)

command is like pkgutil --expand-full /Users/derek/.local/stackql_darwin_multiarch.pkg /Users/derek/.local/stackql

  • /Users/derek/.local/stackql_darwin_multiarch.pkg(archive_file_name): This is the path to your macOS package file.
  • /Users/derek/.local/stackql(unpacked_file_name): This is the directory where the package's contents will be extracted.
    But if I didn't remove unpacked_file_name, there was an error like below,

image

So I add remove existing unpacked_file_name to make a new unpacked_file_name.
After I apply this logic, there are no error like above.

image
(Sorry for debugging message..!)

But if there are any other options instead of deleting unpacked_file_name, let me know any time 😃

3) Removing External Dependencies: Eliminated the reliance on the test.env file during test execution. This enables me to test locally without test.env file

[before]
image

[after]
After I deleted . tests/creds/env_vars/test.env execution logic, there is no error.
Instead, I changed os.environ[VARIABLES] to simple string value.
image

4) Introducing Mocking for Server Mode Tests: Added mocking logic to simulate the behavior of pystackql.StackQL methods in server mode.

[before]
There were errors, because it tried to execute real methods such as pystackql.StackQL.execute, pystackql.stackql.StackQL._run_server_query.
image

[After]
After I mock the method and the response, it doesn't make any error.
image

@jeffreyaven jeffreyaven merged commit efaaea2 into stackql:main Sep 21, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants