decouple torchx native scuba logging and ttfb #1044
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
kiukchung
added a commit
that referenced
this pull request
Apr 16, 2025
Summary: Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Differential Revision: D73060221
kiukchung
force-pushed
the
export-D73060221
branch
from
a85b2cd to
b6b6742
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
kiukchung
added a commit
that referenced
this pull request
Apr 16, 2025
Summary: Pull Request resolved: #1044 Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
kiukchung
force-pushed
the
export-D73060221
branch
from
b6b6742 to
55b0d5a
Compare
facebook-github-bot
pushed a commit
that referenced
this pull request
Apr 16, 2025
Summary: Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
facebook-github-bot
force-pushed
the
export-D73060221
branch
from
55b0d5a to
f2f8ce3
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
hstonec
approved these changes
Apr 16, 2025
kiukchung
added a commit
that referenced
this pull request
Apr 17, 2025
Summary: Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
kiukchung
force-pushed
the
export-D73060221
branch
from
f2f8ce3 to
407739f
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
kiukchung
added a commit
that referenced
this pull request
Apr 17, 2025
Summary: Pull Request resolved: #1044 Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
kiukchung
force-pushed
the
export-D73060221
branch
from
407739f to
486d6be
Compare
facebook-github-bot
pushed a commit
that referenced
this pull request
Apr 18, 2025
Summary: Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
facebook-github-bot
force-pushed
the
export-D73060221
branch
from
486d6be to
66e9d1b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
kiukchung
added a commit
that referenced
this pull request
Apr 19, 2025
Summary: Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
kiukchung
force-pushed
the
export-D73060221
branch
from
66e9d1b to
9196dec
Compare
kiukchung
added a commit
that referenced
this pull request
Apr 19, 2025
Summary: Pull Request resolved: #1044 Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
kiukchung
force-pushed
the
export-D73060221
branch
from
9196dec to
61cd01b
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
Summary: Split out ttfb into its own logging handler and only hook it up to `torchx.runner.events.handler` if the module is included. Also splits out ttfb into its own BUCK target (`//torchx/runner/events/fb:ttfb`) NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb. ## Motivation ttfb pulls unwanted deps such as quickflow (see output of `buck cquery "deps(//torchx/runner/events/fb:ttfb)"`). This prevents us from creating `torchx-lite` - a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively). ## Other Notes Removed the unused `//torchx/runner/events:handlers_oss` targets which was confusing autodeps - hence requiring `autodeps-skip` in `//torchx/runner/events:lib` Reviewed By: hstonec Differential Revision: D73060221
facebook-github-bot
force-pushed
the
export-D73060221
branch
from
61cd01b to
c5485f4
Compare
|
This pull request was exported from Phabricator. Differential Revision: D73060221 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Split out ttfb into its own logging handler and only hook it up to
torchx.runner.events.handlerif the module is included. Also splits out ttfb into its own BUCK target (//torchx/runner/events/fb:ttfb)NOTE: Dependants of //torchx/runner:lib will still get ttfb since I added an manual dep to //torchx/runner/events/fb:ttfb to it. Otherwise users now have to explicitly add a dep to ttfb.
Motivation
ttfb pulls unwanted deps such as quickflow (see output of
buck cquery "deps(//torchx/runner/events/fb:ttfb)"). This prevents us from creatingtorchx-lite- a minimal set of torchx APIs (with no scheduler/workspace implementations pulled transitively).Other Notes
Removed the unused
//torchx/runner/events:handlers_osstargets which was confusing autodeps - hence requiringautodeps-skipin//torchx/runner/events:libDifferential Revision: D73060221