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: Workflow file for persistence performance #326

Conversation

BenjaminBossan
Copy link
Collaborator

  • wrong name
  • trying to fix install

- wrong name
- trying to fix install
@BenjaminBossan
Copy link
Collaborator Author

@skops-dev/maintainers ready for review

@adrinjalali
Copy link
Member

Could you please set this to trigger on PR so that we can test it here and see if the install actually fixes the issue?

@BenjaminBossan
Copy link
Collaborator Author

(don't merge yet)

@adrinjalali While the fundamental issue still exists, I found a "solution" by copying the script to the project root inside the GH action. WDYT?

@adrinjalali
Copy link
Member

Since this is to "fix" import issues, would it work if we do an os.chdir or something inside the script instead, to run it from the root, before running the import statements? I'm not sure which one's better.

But I'm happy to merge and revert once actions/setup-python#634 is resolved maybe?

@BenjaminBossan
Copy link
Collaborator Author

I don't see any advantage either way, so I just left it as is but added a comment to revert the change once the issue is solved. Also removed the PR hook, so this should be ready to be merged.

- name: Install requirements
run: |
pip install .[tests]
pip --version
pip list
- name: Run persistence performance checks
run: python scripts/check_persistence_performance.py
run: |
python3.11 scripts/check_persistence_performance.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this probably can be python3 or even python, right? You can probably revert these two lines to what it was before actually.

All columns together exceed GH column width.
...in an attempt to get the column width down
@BenjaminBossan
Copy link
Collaborator Author

@adrinjalali This works now, here is an output: https://github.com/skops-dev/skops/actions/runs/4531268169/jobs/7981214101?pr=326

I have removed the PR hook again, so we'll be 100% certain in 1 week :)

@adrinjalali adrinjalali merged commit ee7009d into skops-dev:main Mar 27, 2023
@BenjaminBossan BenjaminBossan deleted the fix-persistence-performance-GH-workflow branch March 27, 2023 11:55
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