Skip to content

Feature/gsk 1618 run python tests on windows builder#1382

Merged
andreybavt merged 5 commits intomainfrom
feature/gsk-1618-run-python-tests-on-windows-builder
Sep 12, 2023
Merged

Feature/gsk 1618 run python tests on windows builder#1382
andreybavt merged 5 commits intomainfrom
feature/gsk-1618-run-python-tests-on-windows-builder

Conversation

@Hartorn
Copy link
Copy Markdown
Member

@Hartorn Hartorn commented Sep 6, 2023

Description

Related Issue

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Checklist

  • I've read the CODE_OF_CONDUCT.md document.
  • I've read the CONTRIBUTING.md guide.
  • I've updated the code style using make codestyle.
  • I've written tests for all new methods and classes that I created.
  • I've written the docstring in Google format for all the methods and classes that I used.

@linear
Copy link
Copy Markdown

linear Bot commented Sep 6, 2023

GSK-1618 Run python tests on Windows builder

@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 7 times, most recently from 6660d1a to 0d776dc Compare September 6, 2023 09:40
andreybavt
andreybavt reviewed Sep 6, 2023
View reviewed changes
Comment thread .github/workflows/test-python-client.yml Outdated
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 6 times, most recently from 65fe18d to 18f1d4c Compare September 6, 2023 13:44
Hartorn
Hartorn commented Sep 6, 2023
View reviewed changes
Comment thread python-client/trigger.txt Outdated
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 8 times, most recently from ab3f3f2 to 5b1d640 Compare September 7, 2023 12:15
@Hartorn Hartorn self-assigned this Sep 7, 2023
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 5 times, most recently from 72cab32 to a4389e9 Compare September 7, 2023 15:16
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 4 times, most recently from 6a8f6f6 to 05da514 Compare September 7, 2023 16:53
@Hartorn Hartorn marked this pull request as ready for review September 7, 2023 17:01
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 2 times, most recently from ba88330 to c5b1099 Compare September 8, 2023 08:42
andreybavt
andreybavt reviewed Sep 8, 2023
View reviewed changes
Comment thread python-client/giskard/datasets/base/__init__.py Outdated
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch from c5b1099 to d8c4276 Compare September 8, 2023 13:35
andreybavt
andreybavt reviewed Sep 8, 2023
View reviewed changes
Comment thread .github/workflows/build-python-client.yml Outdated
andreybavt
andreybavt reviewed Sep 8, 2023
View reviewed changes
Comment thread .github/workflows/build-python-client.yml Outdated
andreybavt
andreybavt suggested changes Sep 8, 2023
View reviewed changes
Comment thread .github/workflows/build-python-client.yml

- name: Install dependencies
working-directory: python-client
run: pdm install -G :all
Copy link
Copy Markdown
Contributor

@andreybavt andreybavt Sep 8, 2023

Choose a reason for hiding this comment

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

we don't probably need all groups here (dev for example) do we? prod, test and doc should be fine

Copy link
Copy Markdown
Member Author

@Hartorn Hartorn Sep 8, 2023

Choose a reason for hiding this comment

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

Dev is needed to run linting at least.
Also, I fear that lots of test will fail if I do that, since the split between test and dev deps is not that good yet (ie I just saw pytest-xdist in dev)

Copy link
Copy Markdown
Contributor

@andreybavt andreybavt Sep 8, 2023

Choose a reason for hiding this comment

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

but it can be a good motivation to move the dependencies in the right groups and make sure that people don't make mistakes in the future (for example if you add something to dev and not to test the unit test will fail)

Copy link
Copy Markdown
Member Author

@Hartorn Hartorn Sep 11, 2023

Choose a reason for hiding this comment

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

I will split that into a separated PR then, if it's ok : https://linear.app/giskard/issue/GSK-1717/use-gradle-build-action-instead-of-calling-gradlew

Comment thread .github/workflows/build-python-client.yml
Comment thread .github/workflows/build-python-client.yml
Comment thread .github/workflows/sonar.yml
Comment thread python-client/giskard/datasets/base/__init__.py Outdated
mlflow.log_artifact(local_path)
elif mlflow_client and mlflow_run_id:
mlflow_client.log_artifact(mlflow_run_id, local_path=local_path)
# Note(Bazire): Should we have a raising else here ?
Copy link
Copy Markdown
Contributor

@andreybavt andreybavt Sep 8, 2023

Choose a reason for hiding this comment

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

I'd say yes, to make sure there's no case where one of client or run_id are None
WDYT @rabah-khalek

Comment thread python-client/giskard/datasets/base/__init__.py
Comment thread python-client/tests/fixtures/amazon_review__binary_classification.py Outdated
Comment thread python-client/tests/fixtures/drug_classification__multiclass_classification.py Outdated
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch 2 times, most recently from 613b28a to 916eefb Compare September 8, 2023 15:28
@Hartorn Hartorn force-pushed the feature/gsk-1618-run-python-tests-on-windows-builder branch from 916eefb to 783daeb Compare September 11, 2023 08:45
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Sep 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@Hartorn Hartorn requested a review from andreybavt September 11, 2023 10:04
@andreybavt andreybavt merged commit 8b64257 into main Sep 12, 2023
@Hartorn Hartorn deleted the feature/gsk-1618-run-python-tests-on-windows-builder branch September 13, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@andreybavt andreybavt andreybavt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Hartorn Hartorn

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Hartorn @andreybavt