Purge qhp from git history #2
Assignees
Comments
|
It seems that we'd have to recreate repository to truly purge this stuff. Stale commits in old trees still exist in the repo because github doesn't run gc. |
|
I've purged it from the actual history otherwise. |
bengotow
added a commit
that referenced
this issue
Nov 29, 2019
#1576) Summary: This diff 1) makes it so that the config schema panel appears on brand new Config Editor tabs (previously you had to type a character of a valid key and then it would appear.) and #2) adds a small note to the schema panel that says "Ctrl+Space to show auto-completions inline." There may be better messaging for this but I think knowing that shortcut exists is a game changer and we should get it in ASAP. Test Plan: Updated snapshots Reviewers: #ft, schrockn Reviewed By: #ft, schrockn Subscribers: schrockn Differential Revision: https://dagster.phacility.com/D1509
Closed
This was referenced Oct 18, 2021
2 tasks
clairelin135
added a commit
that referenced
this issue
Oct 7, 2022
test #2 change to set cursor to latest materializations add cli command to migrate job runs from one repo to another (#9376) * add cli command to migrate job runs from one repo to another * extract migration into sql run storage * fix tests * remove prompt_required arg [INTERNAL_BRANCH=prha/run_migration] * rebase; raise; * fix rebase * fix bad rebase [INTERNAL_BRANCH=prha/run_migration]
4 tasks
rexledesma
added a commit
that referenced
this issue
Nov 14, 2023
## Summary & Motivation Following up on #17980, one of the issues with memory usage in `dagster-dbt` is that if the `manifest` is specified as a path, we potentially hold multiple copies of the dictionary manifest in memory when creating our assets. This is because we read from the path to create the dictionary manifest, and then we hold a reference to the manifest dictionary in our asset metadata. When the same path is read multiple times, multiple copies ensure. We can optimize this by implementing a cache when reading from the path, so that the same dictionary manifest is returned even when a manifest path is specified. To further optimize this to hold no reference to the manifest dictionary, we could instead hold a reference to the manifest path in our asset metadata. However, 1. This only works if a manifest path is passed. If a manifest is already specified as a dictionary, then we still have to hold it in memory. This is the worst case. 2. (1) can be solved by preventing a manifest from being specified as a dictionary, but that would be a breaking change. ## How I Tested These Changes Use `tracemalloc`. 1. Create a `jaffle_dagster` project using `dagster-dbt project scaffold` on `jaffle_shop`. 2. Update the code to read the manifest from a path in three separate `@dbt_assets` definitions. 3. Run `PYTHONTRACEMALLOC=1 DAGSTER_DBT_PARSE_PROJECT_ON_LOAD=1 dagster dev` **Before Change** ```diff Top 10 lines #1: <frozen importlib._bootstrap_external>:672: 37114.4 KiB #2: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/text_unidecode/__init__.py:6: 3675.4 KiB _replaces = pkgutil.get_data(__name__, 'data.bin').decode('utf8').split('\x00') - #3: /Users/rexledesma/dagster-labs/dagster/python_modules/libraries/dagster-- dbt/dagster_dbt/dbt_manifest.py:17: 3262.9 KiB - manifest = cast(Mapping[str, Any], orjson.loads(manifest.read_bytes())) #4: /Users/rexledesma/.pyenv/versions/3.10.13/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/abc.py:106: 1313.7 KiB cls = super().__new__(mcls, name, bases, namespace, **kwargs) #5: /Users/rexledesma/.pyenv/versions/3.10.13/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/collections/__init__.py:481: 987.5 KiB result = type(typename, (tuple,), class_namespace) #6: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/mashumaro/core/meta/code/builder.py:114: 978.7 KiB self.globals = globals().copy() #7: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/mashumaro/core/meta/code/builder.py:263: 891.8 KiB exec(code, self.globals, self.__dict__) #8: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/google/protobuf/internal/builder.py:85: 697.9 KiB message_class = _reflection.GeneratedProtocolMessageType( #9: <frozen importlib._bootstrap_external>:128: 655.2 KiB #10: <frozen importlib._bootstrap_external>:1616: 569.6 KiB 53203 other: 38030.8 KiB Total allocated size: 88177.9 KiB ``` **After Change** ```diff Top 10 lines #1: <frozen importlib._bootstrap_external>:672: 37114.9 KiB #2: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/text_unidecode/__init__.py:6: 3675.4 KiB _replaces = pkgutil.get_data(__name__, 'data.bin').decode('utf8').split('\x00') #3: /Users/rexledesma/.pyenv/versions/3.10.13/Library/Frameworks/Python.framework/Versions/3.10 /lib/python3.10/abc.py:106: 1313.7 KiB cls = super().__new__(mcls, name, bases, namespace, **kwargs) + #4: /Users/rexledesma/dagster-labs/dagster/python_modules/libraries/dagster-dbt/dagster_dbt/dbt_manifest.py:13: 1057.4 KiB + return cast(Mapping[str, Any], orjson.loads(manifest_path.read_bytes())) #5: /Users/rexledesma/.pyenv/versions/3.10.13/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/collections/__init__.py:481: 987.5 KiB result = type(typename, (tuple,), class_namespace) #6: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/mashumaro/core/meta/code/builder.py:114: 978.7 KiB self.globals = globals().copy() #7: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/mashumaro/core/meta/code/builder.py:263: 893.6 KiB exec(code, self.globals, self.__dict__) #8: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/google/protobuf/internal/builder.py:85: 697.9 KiB message_class = _reflection.GeneratedProtocolMessageType( #9: <frozen importlib._bootstrap_external>:128: 655.2 KiB #10: <frozen importlib._bootstrap_external>:1616: 569.6 KiB 53195 other: 38013.7 KiB Total allocated size: 85957.5 KiB ``` **Baseline (one `@dbt_assets` definition)** ```diff Top 10 lines #1: <frozen importlib._bootstrap_external>:672: 37114.2 KiB #2: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/text_unidecode/__init__.py:6: 3675.4 KiB _replaces = pkgutil.get_data(__name__, 'data.bin').decode('utf8').split('\x00') #3: /Users/rexledesma/.pyenv/versions/3.10.13/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/abc.py:106: 1313.7 KiB cls = super().__new__(mcls, name, bases, namespace, **kwargs) + #4: /Users/rexledesma/dagster-labs/dagster/python_modules/libraries/dagster-dbt/dagster_dbt/dbt_manifest.py:13: 1056.9 KiB + return cast(Mapping[str, Any], orjson.loads(manifest_path.read_bytes())) #5: /Users/rexledesma/.pyenv/versions/3.10.13/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/collections/__init__.py:481: 987.5 KiB result = type(typename, (tuple,), class_namespace) #6: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/mashumaro/core/meta/code/builder.py:114: 978.7 KiB self.globals = globals().copy() #7: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/mashumaro/core/meta/code/builder.py:263: 893.7 KiB exec(code, self.globals, self.__dict__) #8: /Users/rexledesma/.pyenv/versions/3.10.13/envs/dagster/lib/python3.10/site-packages/google/protobuf/internal/builder.py:85: 697.9 KiB message_class = _reflection.GeneratedProtocolMessageType( #9: <frozen importlib._bootstrap_external>:128: 655.2 KiB #10: <frozen importlib._bootstrap_external>:1616: 569.6 KiB 53175 other: 37976.1 KiB Total allocated size: 85918.9 KiB ```
johannkm
added a commit
that referenced
this issue
Nov 27, 2023
Bug report: https://dagster.slack.com/archives/C04CW71AGBW/p1700237771181059?thread_ts=1697638899.590569&cid=C04CW71AGBW Synopsis: - At definition time, we evaluate @dbt_assets using `DBT_INDIRECT_SELECTION=eager` [side note, I'm kind of surprised this isn't configurable]. - At runtime, if you're using checks, we set `DBT_INDIRECT_SELECTION=empty` so that we have the option to exclude checks if they're not in the selection - If we are in a subset, this works because we explicitly pass the selected models and tests - However, if we determine we're not in a subset, we pass the default selection string, which may not select the tests now that we're not using eager. Solutions: 1. [this diff] If asset checks are enabled, always pass an explicit selection of models and tests 2. The reason we try to use the default selection string when we can is to make things readable from the dbt side. We could try to preserve that by only setting `DBT_INDIRECT_SELECTION=empty` when we're doing a subset and passing the explicit selection, otherwise use eager and the default selection. A concern for (2) is relationship tests. Take the case of two models - `customers,` one non null test - `orders`, one relationship test with customers Dbt will resolve selection `customers` to the `customers` model and both the null test and the relationship test. If you're not using asset checks this is fine, we'll emit observations for both tests. If you are using checks, currently `@dbt_assets(...select="customers")` will only define the null test. This makes sense- we wouldn't even display the relationship test if we ingested it, because we didn't ingest the model its on. With #1, we won't execute the relationship test (which matches the current behavior). With #2 we will whenever we switch over to eager, which is unexpected because we didn't model it. Side note: we should document that when you enable checks, relationship tests on non selected models will no longer run --------- Co-authored-by: Rex Ledesma <[email protected]>
mlarose
added a commit
that referenced
this issue
Feb 21, 2024
OwenKephart
added a commit
that referenced
this issue
Mar 6, 2024
Merged
PedramNavid
added a commit
that referenced
this issue
Aug 20, 2024
# This is the 1st commit message: [docs-beta] Add Vale linter and Sphinx templates # This is the commit message #2: fix tox # This is the commit message #3: add placeholder doc string # This is the commit message #4: Fix broken docs build (#23562) ## Summary & Motivation ## How I Tested These Changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
No description provided.
The text was updated successfully, but these errors were encountered: