opensearch-project / opensearch-py

Python Client for OpenSearch
https://opensearch.org/docs/latest/clients/python/
Apache License 2.0
338 stars 170 forks source link

Added a guide & sample for a custom logger client implementation. #579

Closed Djcarrillo6 closed 10 months ago

Djcarrillo6 commented 10 months ago

Black formatter

Description

Issues Resolved

Resolve proposal related to issue #331

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (e92eac2) 71.98% compared to head (394db91) 71.98%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #579 +/- ## ======================================= Coverage 71.98% 71.98% ======================================= Files 89 89 Lines 7935 7935 ======================================= Hits 5712 5712 Misses 2223 2223 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Djcarrillo6 commented 10 months ago

Hey @dblock @saimedhi I am getting this failure error in the Ci/lint job here. I tried running the black formatter on the file with black --target-version=py33 samples/logging/log_collection_sample.py but that doesn't resolve this import formatting failure. Can you tell me how the imports need to be formatted to resolve it?

dblock commented 10 months ago

@Djcarrillo6 you should run nox -rs format, does that fix the issue?

Djcarrillo6 commented 10 months ago

@dblock I ran the nox -rs format command and it did modify the import order of the sample log file, however I still got an error in the CI/lint job here

dblock commented 10 months ago

@dblock I ran the nox -rs format command and it did modify the import order of the sample log file, however I still got an error in the CI/lint job here

Yes, you need to add types to samples too since #536, there's no automated tool to do that.

Djcarrillo6 commented 10 months ago

@dblock I believe I addressed the lint CI job.. Any advice on how to address this failing integration test here?

dblock commented 10 months ago

@dblock I believe I addressed the lint CI job.. Any advice on how to address this failing integration test here?

I think it's a flake. Will you open a bug for it with details? I restarted it.