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

Update guides-bulk example #774

Merged
merged 1 commit into from
Jul 14, 2024

Conversation

keithrozario
Copy link
Contributor

Fixed examples to make them working python code. (missing commas and quotes.

Description

Example code in the docs were missing crucial commas and double quotes. It meant the code wouldn't run in Python.

While it is a relatively small error that most Python developers will notice it, newcomers might struggle to find the error.

Hope this pull requests makes the example code easier to understand.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

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.

Fixed examples to make them working python code. (missing commas and quotes.



Signed-off-by: Keith Rozario <795867+keithrozario@users.noreply.github.com>
@dblock
Copy link
Member

dblock commented Jul 14, 2024

Thank you!

I assume examples in https://github.com/opensearch-project/opensearch-py/tree/main/samples/bulk do actually run, but if you have a minute please do double-check.

Would love to find a way where the guides are tested, at least for syntax correctness.

@dblock dblock merged commit a380e70 into opensearch-project:main Jul 14, 2024
61 of 70 checks passed
@dblock
Copy link
Member

dblock commented Jul 14, 2024

Link checker is failing, if you have a second to fix that it'd be amazing!

@keithrozario
Copy link
Contributor Author

Thanks for merging this.

  1. I looked through the samples before submitting, and at least they don't have this same issue. But not sure about other issues :)
  2. The linkchecker seems a bit odd though, the link in questions seems to work fine. Perhaps its an issue with the runner while the check was running:

image

@keithrozario
Copy link
Contributor Author

"Would love to find a way where the guides are tested, at least for syntax correctness."

One thing we could do (and we do for other repos as well) is to move all the code to separate .py files instead of inline in the docs. From there we can lint the .py files to ensure syntax correctness. If it's something the project is looking into, I can perhaps help with that as well.

@dblock
Copy link
Member

dblock commented Jul 15, 2024

@keithrozario I think it could be helpful, however I also think people appreciate guides with code snippets in them. We already have working samples. Dunno, maybe it's not worth overthinking.

Another idea is to go the route of opensearch-project/documentation-website#7700 where we generate samples and documentation from spec (opensearch-project/documentation-website#7700), we have these end-to-end tests that run against a real OpenSearch in https://github.com/opensearch-project/opensearch-api-specification/tree/main/tests, they could be turned into working samples and docs mechanically.

dblock pushed a commit to dblock/opensearch-py that referenced this pull request Aug 15, 2024
Fixed examples to make them working python code. (missing commas and quotes.

Signed-off-by: Keith Rozario <795867+keithrozario@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skips changelog verifier
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants