You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "cwegener (via GitHub)" <gi...@apache.org> on 2023/06/26 09:18:59 UTC
[GitHub] [superset] cwegener opened a new pull request, #24514: refactor: pkg_resources -> importlib.metadata
cwegener opened a new pull request, #24514:
URL: https://github.com/apache/superset/pull/24514
pkg_resources is in the process of being deprecated in favor of
importlib.metadata and importlib.resources
importlib.metadata has been available in the standard library since 3.8
And as of 3.10 the importlib.metadata API is no longer provisional
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610281814
@cwegener I gather you mean `install_requirements` rather than `install_dependencies`? Note that `pip-compile-multi` should adhere to the requirements. Would you mind sharing the command(s) you ran?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610363915
@cwegener with regards to,
> I hope to convince the superset contributors to migrate from setuptools+pip-compile-multi to hatch+pdm ...
it might make sense to first create an issue (or possibly a SIP dependency on the complexity) regarding your intentions as it's probably worth discussing the pros/cons especially given the slew of Python package managers floating around.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610307262
I did just force push the 'noop' result from pip-compile-multi, which is basically just adding the comment into the requirements/base.txt for good measure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1611507991
@cwegener after rereading your PR description you mention that,
> importlib.metadata has been available in the standard library since 3.8
and given we only support Python 3.9+ I was wondering why we need to use the backport.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1612196832
> Specifically, the `entry_points()` signature in 3.9 was still different, it didn't have the `groups` parameter in the 3.9 standard library. So the stable API is only available in the backport. (Found that out when I developed this change in 3.10 accidentally and then ran unit tests in 3.9)
The best I can tell, the 'select' feature wasn't there in the `entry_points()`
I did just see though that there also is a more specific backport of JUST the select feature for entry_points() :grin:
https://github.com/jaraco/backports.entry_points_selectable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley merged pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley merged PR #24514:
URL: https://github.com/apache/superset/pull/24514
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1609907219
Thanks @cwegener for the PR. Note you'll also have to re-compile the Python dependencies which ensures we have a frozen set of Python packages (and sub-packages) for testing purposes. Please refer to the CONTRIBUTING.md file with regards to how to do this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on a diff in pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on code in PR #24514:
URL: https://github.com/apache/superset/pull/24514#discussion_r1244436061
##########
requirements/base.txt:
##########
@@ -137,7 +137,9 @@ humanize==3.11.0
idna==3.2
# via email-validator
importlib-metadata==6.6.0
- # via flask
+ # via
Review Comment:
Yes!
##########
requirements/base.txt:
##########
@@ -137,7 +137,9 @@ humanize==3.11.0
idna==3.2
# via email-validator
importlib-metadata==6.6.0
- # via flask
+ # via
+ # apache-superset
Review Comment:
Yes!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610250632
> Thanks @cwegener for the PR. Note you'll also have to re-compile the Python dependencies which ensures we have a frozen set of Python packages (and sub-packages) for testing purposes. Please refer to the CONTRIBUTING.md file with regards to how to do this.
Hi John. I tried, but pip-compile-multi seems to just blissfully ignore the install_dependencies from setup.py
Do you have any hints on how to make pip-compile-multi read the install_dependencies collection from setup.py?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610309457
On a different note, once I'm done with this `pkg_resources` migration (I'll publish the other part which is the migration to importlib.resources shortly) and setuptools should then no longer be a dependency,I hope to convince the superset contributors to migrate from setuptools+pip-compile-multi to hatch+pdm ... :smile:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610313873
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#24514](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (b5d27fe) into [master](https://app.codecov.io/gh/apache/superset/commit/ba3bdc077cfe1a017105332dc688b9d7faef6795?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ba3bdc0) will **decrease** coverage by `10.99%`.
> The diff coverage is `94.73%`.
> :exclamation: Current head b5d27fe differs from pull request most recent head 7a9ec03. Consider uploading reports for the commit 7a9ec03 to get more accurate results
```diff
@@ Coverage Diff @@
## master #24514 +/- ##
===========================================
- Coverage 69.06% 58.07% -10.99%
===========================================
Files 1901 1903 +2
Lines 74019 74027 +8
Branches 8116 8118 +2
===========================================
- Hits 51121 42992 -8129
- Misses 20787 28924 +8137
Partials 2111 2111
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.79% <100.00%> (-0.01%)` | :arrow_down: |
| python | `60.59% <100.00%> (-22.90%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `54.65% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [.../plugins/plugin-chart-echarts/src/BoxPlot/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQm94UGxvdC9pbmRleC50cw==) | `50.00% <ø> (ø)` | |
| [...d/plugins/plugin-chart-echarts/src/Funnel/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvRnVubmVsL2luZGV4LnRz) | `50.00% <ø> (ø)` | |
| [...nd/plugins/plugin-chart-echarts/src/Gauge/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR2F1Z2UvaW5kZXgudHM=) | `50.00% <ø> (ø)` | |
| [...nd/plugins/plugin-chart-echarts/src/Graph/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvR3JhcGgvaW5kZXgudHM=) | `50.00% <ø> (ø)` | |
| [.../plugin-chart-echarts/src/MixedTimeseries/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL2luZGV4LnRz) | `25.00% <ø> (ø)` | |
| [...tend/plugins/plugin-chart-echarts/src/Pie/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvUGllL2luZGV4LnRz) | `50.00% <ø> (ø)` | |
| [...nd/plugins/plugin-chart-echarts/src/Radar/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvUmFkYXIvaW5kZXgudHM=) | `50.00% <ø> (ø)` | |
| [...plugins/plugin-chart-echarts/src/Sunburst/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvU3VuYnVyc3QvaW5kZXgudHM=) | `50.00% <ø> (ø)` | |
| [.../plugin-chart-echarts/src/Timeseries/Area/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy9BcmVhL2luZGV4LnRz) | `33.33% <ø> (ø)` | |
| [...-chart-echarts/src/Timeseries/Regular/Bar/index.ts](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy9SZWd1bGFyL0Jhci9pbmRleC50cw==) | `33.33% <ø> (ø)` | |
| ... and [23 more](https://app.codecov.io/gh/apache/superset/pull/24514?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [297 files with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/24514/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610298091
> @cwegener I gather you mean `install_requirements` rather than `install_dependencies`? Note that `pip-compile-multi` should adhere to the requirements. Would you mind sharing the command(s) you ran?
Sorry, yes. Typo. `install_requires` is what I'm referring to.
All good. It turns out that the result of `pip-compile-multi -P importlib-metadata` is simply a noop, because there is no change to the version of `importlib-metadata` that is already present.
And `pip-compile-multi` just doesn't need to know that `importlib-medata` is now a top-level require instead of a transitive one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1610355832
@john-bodley Sorry about the missing import sort. I just pushed the fix for that. Can you run the GH actions again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] cwegener commented on pull request #24514: refactor: pkg_resources -> importlib.metadata
Posted by "cwegener (via GitHub)" <gi...@apache.org>.
cwegener commented on PR #24514:
URL: https://github.com/apache/superset/pull/24514#issuecomment-1612189254
> and given we only support Python 3.9+ I was wondering why we need to use the backport.
The backport is needed because the API of `importlib.metadata` is actually different in 3.9. The API has only been stable in 3.10.
Specifically, the `entry_points()` signature in 3.9 was still different, it didn't have the `groups` parameter in the 3.9 standard library. So the stable API is only available in the backport. (Found that out when I developed this change in 3.10 accidentally and then ran unit tests in 3.9)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org