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

Add Thrift and Hive to NOTICE #410

Merged
merged 3 commits into from
Feb 12, 2024
Merged

Add Thrift and Hive to NOTICE #410

merged 3 commits into from
Feb 12, 2024

Conversation

Fokko
Copy link
Contributor

@Fokko Fokko commented Feb 11, 2024

No description provided.

@danielcweeks
Copy link
Contributor

danielcweeks commented Feb 11, 2024

@Fokko I'm not sure this is necessary since we don't actually bundle anything in the project. Per the ASF site

Only bundled bits matter

This is a little confusing because we did generate code based off those thrift definitions, but I don't think that really qualifies since we're not including anything that originated from those projects.

@Fokko
Copy link
Contributor Author

Fokko commented Feb 11, 2024

@danielcweeks I just replied on the mailing list. We do ship the Python-generated Thrift definitions that are stored here: https://github.com/apache/iceberg-python/tree/main/vendor

For example, the ttypes.py is directly from the Thrift project. For the Hive case, it is a bit more ambiguous since we ship the Python-generated code from the Hive definition, but not the definition itself.

@Fokko Fokko added this to the PyIceberg 0.6.0 release milestone Feb 12, 2024
@Fokko Fokko merged commit e9e265a into main Feb 12, 2024
6 checks passed
@Fokko Fokko deleted the fd-notice branch February 12, 2024 10:05
@rdblue
Copy link
Contributor

rdblue commented Feb 12, 2024

@Fokko, I think that this change is incorrect. It conflicts with the conventions that we use in Avro, Parquet, and the other Iceberg releases. LICENSE is where we put license information for code. NOTICE is for legally required notifications only. Here's the documentation on NOTICE:

Modifications to NOTICE

The NOTICE file is reserved for a certain subset of legally required notifications which are not satisfied by either the text of LICENSE or the presence of licensing information embedded within the bundled dependency. Aside from Apache-licensed dependencies which supply NOTICE files of their own, it is uncommon for a dependency to require additions to NOTICE.

Copyright notifications which have been relocated, rather than removed, from source files must be preserved in NOTICE. However, elements such as the copyright notifications embedded within BSD and MIT licenses do not need to be duplicated in NOTICE. You can leave those notices in their original locations.

It is important to keep NOTICE as brief and simple as possible, as each addition places a burden on downstream consumers.

Do not add anything to NOTICE which is not legally required.

Legally required notifications are things like attribution required by third-party licenses. We also include sections of other NOTICE files that need to be preserved because they apply to this project as well as the original project.

License information should be included in the LICENSE file because it is the canonical place for license attribution. That's why the documentation states that NOTICE is for "notifications which are not satisfied by either the text of LICENSE or ..."

It's fine if we don't ship code and should update LICENSE to reflect that, but we can't add this information to NOTICE.

Fokko added a commit to Fokko/iceberg-python that referenced this pull request Feb 12, 2024
Fokko added a commit to Fokko/iceberg-python that referenced this pull request Feb 13, 2024
Fokko added a commit to Fokko/iceberg-python that referenced this pull request Feb 13, 2024
rdblue pushed a commit that referenced this pull request Feb 13, 2024
* Revert "Add Thrift and Hive to NOTICE (#410)"

This reverts commit e9e265a.

* Update year
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants