-
-
Notifications
You must be signed in to change notification settings - Fork 23
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
✨ anomalist: improve utils #3385
Conversation
Quick links (staging server):
Login: chart-diff: ✅No charts for review.Edited: 2024-10-08 14:45:06 UTC |
return dataset_id | ||
|
||
|
||
def get_variables_in_dataset( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually, I think we should move this as a method to grapher_model.Dataset
so that all dataset-related methods are in one place. (Not now though!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes sense. I've added a @deprecated tag
|
||
|
||
def get_variables_in_dataset( | ||
dataset_id: int, only_used_in_charts: bool = False, db_conn: Optional[pymysql.Connection] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
db_conn
should eventually also get deprecated. We should only use engine
or session
and read_sql
function from db.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes sense. I've added a @deprecated tag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, ship it!
* ✨ wizard: anomalies * wip * bump streamlit * wip * wip: chart * wip * todo * plot indicator * re-structure * wip: loading indicators * fix API grapher_chart * deprecate chart_html * chart_html -> grapher_chart * clean * ci/cd * wip * wip * changed module name * custom components module * add methods to get uris * new alias * get dataset uris * update import * update gpt pricing * update import * wip * provide entity-context for anomaly * wip: anomalist v2 * wip * wip * lock * ✨ anomalist: improve utils (#3385) * wip * db -> db_utils * io -> db * move things db_utils -> db * db -> grapher_io * db -> grapher_io, db_utils -> db * docstring * db_utils -> db * wip * remove indicator * add overloads * ci/cd * wip * cicd * wip * deprecation warnings * missing import * hide anomalist in wizard
* ✨ wizard: anomalies * wip * bump streamlit * wip * wip: chart * wip * todo * plot indicator * re-structure * wip: loading indicators * fix API grapher_chart * deprecate chart_html * chart_html -> grapher_chart * clean * ci/cd * wip * wip * changed module name * custom components module * add methods to get uris * new alias * get dataset uris * update import * update gpt pricing * update import * wip * provide entity-context for anomaly * wip: anomalist v2 * wip * wip * lock * ✨ anomalist: improve utils (#3385) * wip * db -> db_utils * io -> db * move things db_utils -> db * db -> grapher_io * db -> grapher_io, db_utils -> db * docstring * db_utils -> db * wip * remove indicator * add overloads * ci/cd * wip * cicd * wip * deprecation warnings * missing import * hide anomalist in wizard
This PR re-organizes several grapher/db/s3 utils in our library.
We had some tools spread in multiple modules, within
etl/
and inapps/
. The idea is to compile these in centralised modules, so that it is easier to improve them, their maintenance, and enhancement.Changes include:
etl.db
: This module already existed but included a mixture of database utils and methods to query specific bits of data from our database. From now on, it should only include database utils.etl.grapher_io
: This is a new module, which should contain all our tools to access data/metadata (and possibly other bits of content) from our Grapher database/api (mysql/s3). The important bit here, is that it accesses non-etl content. Its content comes from various places: new content, frometl.db
, from,apps.backport.datasync.data_metadata
, etc.etl.grapher_model
: Added some functions to some ORM objects. For instance, classVariable
has now aget_data
method to access it's data (i.e. it accesses s3 data).