-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Replace "--no-conda"
with "--env-manager", "local"
in tests
#5604
Conversation
Signed-off-by: nishipy <goodisonev4@gmail.com>
Signed-off-by: nishipy <goodisonev4@gmail.com>
@nishipy Thanks for the PR! Can you also fix this line? |
tests/helper_functions.py
Outdated
@@ -158,7 +158,7 @@ def pyfunc_serve_and_score_model( | |||
:param activity_polling_timeout_seconds: The amount of time, in seconds, to wait before | |||
declaring the scoring process to have failed. | |||
:param extra_args: A list of extra arguments to pass to the pyfunc scoring server command. For | |||
example, passing ``extra_args=["--no-conda"]`` will pass the ``--no-conda`` | |||
example, passing ``extra_args=["--env-manager", "local"]`` will pass the ``--env-manager local`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pylint complains this is too long
https://github.com/mlflow/mlflow/runs/5830151998?check_suite_focus=true#step:7:20
tests/helper_functions.py:161:0: C0301: Line too long (119/100) (line-too-long)
Can you adjust the line length by wrapping it?
Hi @harupy, |
Signed-off-by: nishipy <goodisonev4@gmail.com>
Signed-off-by: nishipy <goodisonev4@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (if all the CI checks pass)!
@nishipy Thanks for working on this PR, would you be interested in making another contribution? |
@nishipy Awesome, our team found this GitHub feature that annotates code with lint warnings/errors like the image below: https://github.com/actions/toolkit/blob/main/docs/problem-matchers.md#problem-matchers In the attached image, javascript files are annotated. We want to do the same thing for python scripts. MotivationHere's a brief comparison of how we check lint errors between with and without this feature: Now
If we use this feature
What needs to be done?
mlflow/.github/workflows/master.yml Line 37 in dc3758c
Please let me know if you're interested in this. I'll provide more details if you're. |
Actually it is the first time for me to see this GitHub feature, but it looks so interesting and useful for us. Let me know more details. |
Of course, I described what needs to be done in my comment above. I think we can test this feature with pylint. |
@nishipy Let me know if you have questions. |
Thanks for the information. I will try it. |
@nishipy No, we don't have to :) |
@nishipy We can enable problem matchers like this: - name: Register problem matchers
run: |
echo "::add-matcher::.github/workflows/matchers/pylint.json" |
@harupy Thanks! If I understand correctly, we have to implement the following things:
|
@nishipy Yes, exactly! |
Got it! I will start to do that later. |
What changes are proposed in this pull request?
Fix #5590.
How is this patch tested?
Existing tests
Does this PR change the documentation?
ci/circleci: build_doc
check. If it's successful, proceed to thenext step, otherwise fix it.
Details
on the right to open the job page of CircleCI.Artifacts
tab.docs/build/html/index.html
.Release Notes
Is this a user-facing change?
(Details in 1-2 sentences. You can just refer to another PR with a description if this PR is part of a larger change.)
What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/artifacts
: Artifact stores and artifact loggingarea/build
: Build and test infrastructure for MLflowarea/docs
: MLflow documentation pagesarea/examples
: Example codearea/model-registry
: Model Registry service, APIs, and the fluent client calls for Model Registryarea/models
: MLmodel format, model serialization/deserialization, flavorsarea/projects
: MLproject format, project running backendsarea/scoring
: MLflow Model server, model deployment tools, Spark UDFsarea/server-infra
: MLflow Tracking server backendarea/tracking
: Tracking Service, tracking client APIs, autologgingInterface
area/uiux
: Front-end, user experience, plotting, JavaScript, JavaScript dev serverarea/docker
: Docker use across MLflow's components, such as MLflow Projects and MLflow Modelsarea/sqlalchemy
: Use of SQLAlchemy in the Tracking Service or Model Registryarea/windows
: Windows supportLanguage
language/r
: R APIs and clientslanguage/java
: Java APIs and clientslanguage/new
: Proposals for new client languagesIntegrations
integrations/azure
: Azure and Azure ML integrationsintegrations/sagemaker
: SageMaker integrationsintegrations/databricks
: Databricks integrationsHow should the PR be classified in the release notes? Choose one:
rn/breaking-change
- The PR will be mentioned in the "Breaking Changes" sectionrn/none
- No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" sectionrn/feature
- A new user-facing feature worth mentioning in the release notesrn/bug-fix
- A user-facing bug fix worth mentioning in the release notesrn/documentation
- A user-facing documentation change worth mentioning in the release notes