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

Data Processor: Resolve several bugs found while publishing a Studio #19309

Merged
merged 46 commits into from
Jan 18, 2024

Conversation

tchaton
Copy link
Contributor

@tchaton tchaton commented Jan 18, 2024

What does this PR do?

This PR resolves some issues:

  • upstream the resolver from lightning-cloud to ease the development of its functionalities.
  • adds support for partial function so the published Studio is as clean as possible
  • tiny fixes there and there

Fixes #<issue_number>

Before submitting
  • Was this discussed/agreed via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

Reviewer checklist
  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

cc @Borda

Copy link

gitguardian bot commented Jan 18, 2024

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
5685611 Generic High Entropy Secret c10c882 tests/tests_data/streaming/test_resolver.py View secret
5685611 Generic High Entropy Secret c10c882 tests/tests_data/streaming/test_resolver.py View secret
5685611 Generic High Entropy Secret c10c882 tests/tests_data/streaming/test_resolver.py View secret
5685611 Generic High Entropy Secret c10c882 tests/tests_data/streaming/test_resolver.py View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@github-actions github-actions bot added app (removed) Generic label for Lightning App package dependencies Pull requests that update a dependency file data (external) litdata package labels Jan 18, 2024
@tchaton tchaton changed the title Data Processor: Some fixes to enable Data Processor: Enable partial functions Jan 18, 2024
@tchaton tchaton changed the title Data Processor: Enable partial functions Data Processor: Resolve several bugs found while publishing a Studio Jan 18, 2024
@tchaton tchaton marked this pull request as ready for review January 18, 2024 16:01
Copy link
Contributor

github-actions bot commented Jan 18, 2024

⚡ Required checks status: All passing 🟢

Groups summary

🟢 lightning_data: CPU workflow
Check ID Status
data-cpu (macOS-11, lightning, 3.10, 2.1) success
data-cpu (ubuntu-20.04, lightning, 3.10, 2.1) success
data-cpu (windows-2022, lightning, 3.10, 2.1) success

These checks are required after the changes to src/lightning/data/__init__.py, src/lightning/data/streaming/cache.py, src/lightning/data/streaming/combined.py, src/lightning/data/streaming/constants.py, src/lightning/data/streaming/data_processor.py, src/lightning/data/streaming/dataset.py, src/lightning/data/streaming/functions.py, src/lightning/data/streaming/resolver.py, tests/tests_data/streaming/test_data_processor.py, tests/tests_data/streaming/test_dataset.py, tests/tests_data/streaming/test_functions.py, tests/tests_data/streaming/test_reader.py, tests/tests_data/streaming/test_resolver.py.

🟢 lightning_app: Tests workflow
Check ID Status
app-pytest (macOS-11, lightning, 3.8, latest) success
app-pytest (macOS-11, lightning, 3.8, oldest) success
app-pytest (macOS-11, app, 3.9, latest) success
app-pytest (macOS-12, app, 3.11, latest) success
app-pytest (ubuntu-20.04, lightning, 3.8, latest) success
app-pytest (ubuntu-20.04, lightning, 3.8, oldest) success
app-pytest (ubuntu-20.04, app, 3.9, latest) success
app-pytest (ubuntu-22.04, app, 3.11, latest) success
app-pytest (windows-2022, lightning, 3.8, latest) success
app-pytest (windows-2022, lightning, 3.8, oldest) success
app-pytest (windows-2022, app, 3.8, latest) success
app-pytest (windows-2022, app, 3.11, latest) success

These checks are required after the changes to requirements/app/app.txt.

🟢 lightning_app: Examples
Check ID Status
app-examples (macOS-11, lightning, 3.9, latest) success
app-examples (macOS-11, lightning, 3.9, oldest) success
app-examples (macOS-11, app, 3.9, latest) success
app-examples (ubuntu-20.04, lightning, 3.9, latest) success
app-examples (ubuntu-20.04, lightning, 3.9, oldest) success
app-examples (ubuntu-20.04, app, 3.9, latest) success
app-examples (windows-2022, lightning, 3.9, latest) success
app-examples (windows-2022, lightning, 3.9, oldest) success
app-examples (windows-2022, app, 3.9, latest) success

These checks are required after the changes to requirements/app/app.txt.

🟢 lightning_app: Docs
Check ID Status
docs-make (app, doctest) success
docs-make (app, html) success

These checks are required after the changes to requirements/app/app.txt.

🟢 mypy
Check ID Status
mypy success

These checks are required after the changes to requirements/app/app.txt, src/lightning/data/__init__.py, src/lightning/data/streaming/cache.py, src/lightning/data/streaming/combined.py, src/lightning/data/streaming/constants.py, src/lightning/data/streaming/data_processor.py, src/lightning/data/streaming/dataset.py, src/lightning/data/streaming/functions.py, src/lightning/data/streaming/resolver.py.

🟢 install
Check ID Status
install-pkg (ubuntu-22.04, app, 3.8) success
install-pkg (ubuntu-22.04, app, 3.11) success
install-pkg (ubuntu-22.04, fabric, 3.8) success
install-pkg (ubuntu-22.04, fabric, 3.11) success
install-pkg (ubuntu-22.04, pytorch, 3.8) success
install-pkg (ubuntu-22.04, pytorch, 3.11) success
install-pkg (ubuntu-22.04, lightning, 3.8) success
install-pkg (ubuntu-22.04, lightning, 3.11) success
install-pkg (ubuntu-22.04, notset, 3.8) success
install-pkg (ubuntu-22.04, notset, 3.11) success
install-pkg (macOS-12, app, 3.8) success
install-pkg (macOS-12, app, 3.11) success
install-pkg (macOS-12, fabric, 3.8) success
install-pkg (macOS-12, fabric, 3.11) success
install-pkg (macOS-12, pytorch, 3.8) success
install-pkg (macOS-12, pytorch, 3.11) success
install-pkg (macOS-12, lightning, 3.8) success
install-pkg (macOS-12, lightning, 3.11) success
install-pkg (macOS-12, notset, 3.8) success
install-pkg (macOS-12, notset, 3.11) success
install-pkg (windows-2022, app, 3.8) success
install-pkg (windows-2022, app, 3.11) success
install-pkg (windows-2022, fabric, 3.8) success
install-pkg (windows-2022, fabric, 3.11) success
install-pkg (windows-2022, pytorch, 3.8) success
install-pkg (windows-2022, pytorch, 3.11) success
install-pkg (windows-2022, lightning, 3.8) success
install-pkg (windows-2022, lightning, 3.11) success
install-pkg (windows-2022, notset, 3.8) success
install-pkg (windows-2022, notset, 3.11) success

These checks are required after the changes to src/lightning/data/__init__.py, src/lightning/data/streaming/cache.py, src/lightning/data/streaming/combined.py, src/lightning/data/streaming/constants.py, src/lightning/data/streaming/data_processor.py, src/lightning/data/streaming/dataset.py, src/lightning/data/streaming/functions.py, src/lightning/data/streaming/resolver.py, requirements/app/app.txt.


Thank you for your contribution! 💜

Note
This comment is automatically generated and updates for 60 minutes every 180 seconds. If you have any other questions, contact carmocca for help.

Copy link

codecov bot commented Jan 18, 2024

Codecov Report

Merging #19309 (3893894) into master (93c1ab0) will decrease coverage by 32%.
The diff coverage is 85%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19309      +/-   ##
==========================================
- Coverage      84%      51%     -32%     
==========================================
  Files         445      440       -5     
  Lines       37332    37404      +72     
==========================================
- Hits        31182    19135   -12047     
- Misses       6150    18269   +12119     

src/lightning/data/streaming/resolver.py Show resolved Hide resolved
src/lightning/data/streaming/resolver.py Outdated Show resolved Hide resolved
src/lightning/data/streaming/resolver.py Show resolved Hide resolved
src/lightning/data/streaming/resolver.py Show resolved Hide resolved
@mergify mergify bot added the ready PRs ready to be merged label Jan 18, 2024
@tchaton tchaton merged commit 97d71ab into master Jan 18, 2024
88 checks passed
@tchaton tchaton deleted the some_fixes branch January 18, 2024 20:46
@awaelchli awaelchli added this to the 2.1.x milestone Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app (removed) Generic label for Lightning App package data (external) litdata package dependencies Pull requests that update a dependency file ready PRs ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants