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

Fixes #17221: Setuptools Version Fix, Resolve Ingestion Setup Failure #17216

Merged
merged 6 commits into from
Jul 29, 2024

Conversation

ayush-shah
Copy link
Member

@ayush-shah ayush-shah commented Jul 29, 2024

Describe your changes:

Fixes #17221 Setuptools Issue

ref: pypa/setuptools#4519

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Copy link
Contributor

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and has passed the active policy.

View this project in the Phylum UI

Copy link
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

Copy link
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

Copy link
Contributor

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@ayush-shah ayush-shah changed the title Fix setuptools version bound as it fails ingestion setup Setuptools Version Fix: Resolve Ingestion Setup Failure Jul 29, 2024
@ayush-shah ayush-shah changed the title Setuptools Version Fix: Resolve Ingestion Setup Failure Fixes #17221: Setuptools Version Fix, Resolve Ingestion Setup Failure Jul 29, 2024

.PHONY: install_apis
install_apis: ## Install the REST APIs module to the current environment
python -m pip install $(ROOT_DIR)/openmetadata-airflow-apis/ setuptools==69.0.2
PIP_CONSTRAINT=$(ROOT_DIR)/openmetadata-airflow-apis/constraints.txt python -m pip install $(ROOT_DIR)/openmetadata-airflow-apis/ setuptools==69.0.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

why are we adding these constraints here? Can't we handle it from the setup file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

or in pyproject.toml I think we have a requires https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/pyproject.toml#L2

I'd like to avoid having to look at versions to yet another place

Copy link
Member Author

Choose a reason for hiding this comment

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

tried different solutions here, pyproject.toml setuptools version modification didn't work as expected, we already have that in place and it is still failing, the reach for requires doesn't impact 3rd party packages as far as i could test for the setuptools issue

Copy link
Member Author

Choose a reason for hiding this comment

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

adding setuptools in depedencies within setup.py didn't work as expected

Copy link
Member Author

Choose a reason for hiding this comment

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

only solution that could impact 3rd party packages was the constraint one

Copy link
Member Author

Choose a reason for hiding this comment

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

There is one more solution that i have yet to try i.e. --no-build-isolation

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok thanks, let's keep it as-is until setuptools fixes it on their end

@ayush-shah ayush-shah marked this pull request as ready for review July 29, 2024 08:58
@ayush-shah ayush-shah requested a review from a team as a code owner July 29, 2024 08:58
Copy link

Quality Gate Passed Quality Gate passed for 'open-metadata-airflow-apis'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

@ayush-shah ayush-shah merged commit 1962ffa into main Jul 29, 2024
22 of 26 checks passed
@ayush-shah ayush-shah deleted the fix-setup-failure branch July 29, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ingestion safe to test Add this label to run secure Github workflows on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Ingestion Packages and CI Pipelines breaking due to latest setuptools breaking change
2 participants