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

chore: update core deps #25532

Merged
merged 4 commits into from
Nov 12, 2024
Merged

chore: update core deps #25532

merged 4 commits into from
Nov 12, 2024

Conversation

praveen-influx
Copy link
Contributor

@praveen-influx praveen-influx commented Nov 11, 2024

  • arrow/parquet deps are patched (as in core)
  • three specific code changes to cope with changes in core crates
    • TransitionPartitionId, use from_parts instead of new
    • arrow buffers can take &[u8] directly without to_vec()/vec!
      (used only in tests)
    • schema and influxdb_line_protocol crates need v3 feature enabled

@praveen-influx praveen-influx marked this pull request as draft November 11, 2024 17:47
@praveen-influx praveen-influx force-pushed the chore/update-core-deps branch 2 times, most recently from f807b55 to d67e837 Compare November 12, 2024 09:31
- arrow/parquet deps are patched (as in core)
- three specific code changes to cope with changes in core crates
  - TransitionPartitionId, use `from_parts` instead of `new`
  - arrow buffers can take &[u8] directly without `to_vec()`/`vec!`
    (used only in tests)
  - `schema` and `influxdb_line_protocol` crates need `v3` feature enabled
Unicode-3.0 license is added to allowed licenses list, without it
end up with 19 errors (`zerovec`, `zerovec-derive` etc.)
@praveen-influx praveen-influx marked this pull request as ready for review November 12, 2024 09:40
@praveen-influx praveen-influx requested review from hiltontj, pauldix, mgattozzi and jdstrand and removed request for pauldix November 12, 2024 09:40
@@ -405,24 +405,6 @@ impl Database {

#[async_trait]
impl QueryNamespace for Database {
async fn chunks(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method has been removed from QueryNamespace trait

Copy link
Contributor

@hiltontj hiltontj left a comment

Choose a reason for hiding this comment

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

Reviewing from my phone and had one comment but I'll take another look when I'm at my desk.

@@ -9,7 +9,7 @@ license.workspace = true
# Core Crates
influxdb-line-protocol.workspace = true
observability_deps.workspace = true
schema.workspace = true
schema = { workspace = true, features = ["v3"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to keep the feature set at the workspace level, then don't need to set it in each individual cargo file.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, it was previously set in the workspace Cargo.toml. Having it there could save someone a few head scratches if they pull one of the v3 featured crates and find things broken, and I'm not sure we would ever have a crate that depends on one of these and does not use the feature.

Copy link
Contributor

@jdstrand jdstrand left a comment

Choose a reason for hiding this comment

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

I commented with 2 questions.

deny.toml Show resolved Hide resolved
deny.toml Show resolved Hide resolved
- move enabling v3 feature to root Cargo.toml
- added the upstream PR for datafusion-common that introduced RUSTSEC-2024-0384
Copy link
Contributor

@jdstrand jdstrand left a comment

Choose a reason for hiding this comment

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

Based on answers to my questions, approving the deny.toml changes.

Copy link
Contributor

@hiltontj hiltontj left a comment

Choose a reason for hiding this comment

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

🚢

@praveen-influx praveen-influx merged commit 814eb31 into main Nov 12, 2024
13 checks passed
@hiltontj hiltontj added the v3 label Nov 14, 2024
mgattozzi pushed a commit that referenced this pull request Feb 10, 2025
* chore: update core deps

- arrow/parquet deps are patched (as in core)
- three specific code changes to cope with changes in core crates
  - TransitionPartitionId, use `from_parts` instead of `new`
  - arrow buffers can take &[u8] directly without `to_vec()`/`vec!`
    (used only in tests)
  - `schema` and `influxdb_line_protocol` crates need `v3` feature enabled

* chore: update deny.toml

* chore: formatting and deny toml changes

Unicode-3.0 license is added to allowed licenses list, without it
end up with 19 errors (`zerovec`, `zerovec-derive` etc.)

* chore: address PR feedback

- move enabling v3 feature to root Cargo.toml
- added the upstream PR for datafusion-common that introduced RUSTSEC-2024-0384
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants