Skip to content

Add Connector API #2531

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

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Add Connector API #2531

merged 1 commit into from
Apr 23, 2024

Conversation

pquentin
Copy link
Member

Each new API unfortunately requires manual steps.

Copy link

A documentation preview will be available soon.

Request a new doc build by commenting
  • Rebuild this PR: run docs-build
  • Rebuild this PR and all Elastic docs: run docs-build rebuild

run docs-build is much faster than run docs-build rebuild. A rebuild should only be needed in rare situations.

If your PR continues to fail for an unknown reason, the doc build pipeline may be broken. Elastic employees can check the pipeline status here.

Copy link
Contributor

@miguelgrinberg miguelgrinberg left a comment

Choose a reason for hiding this comment

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

The change looks good, but I have questions:

  1. Why are we not adding tests for this new API?
  2. How does the spec fit in this? It seems this is an API that has been added manually. When will we be able to auto-generate this code from a common spec?
  3. What's up with the builds, which have lots of connection errors, report 14% coverage, and report success?

@pquentin
Copy link
Member Author

  1. Tests need to be added there: https://github.com/elastic/elasticsearch-clients-tests. (But I have to switch from the Elasticsearch server tests to those first.)
  2. The Python code generator is limited: it only generated the connector.py classes, but everything else (connector.py imports, exposing this new namespace to the rest of the client, and docs) is manual. This work is only needed when there's a new namespace, but it happens a lot these days even though we "only" have ~40 namespaces.
  3. The build is separated in two: the tests that don't need an Elasticsearch server run in GitHub, and the one that do are skipped in GitHub and only run in Buildkite. The real server tests are missing for now, but they add about 40 minutes of CI time currently. I hope that https://github.com/elastic/elasticsearch-clients-tests will be much faster.

@pquentin pquentin merged commit dd6c015 into elastic:main Apr 23, 2024
@pquentin pquentin deleted the connector-api branch April 23, 2024 07:35
github-actions bot pushed a commit that referenced this pull request Apr 23, 2024
(cherry picked from commit dd6c015)
pquentin added a commit that referenced this pull request Apr 23, 2024
(cherry picked from commit dd6c015)

Co-authored-by: Quentin Pradet <[email protected]>
pquentin added a commit to pquentin/elasticsearch-py that referenced this pull request May 6, 2024
pquentin added a commit that referenced this pull request May 6, 2024
pquentin added a commit that referenced this pull request May 14, 2024
pquentin added a commit that referenced this pull request May 14, 2024
pquentin added a commit to pquentin/elasticsearch-py that referenced this pull request Aug 1, 2024
@pquentin pquentin mentioned this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants