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

feat: adding Progress Bar to Benchmark script #15719

Merged
merged 8 commits into from
Jul 19, 2021

Conversation

AAfghahi
Copy link
Member

@AAfghahi AAfghahi commented Jul 15, 2021

SUMMARY

This adds a progress bar to the current benchmark script.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2021-07-15 at 3 43 48 PM

Untitled.mov

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@AAfghahi AAfghahi force-pushed the arash/progressBar branch 4 times, most recently from 1e89172 to a455019 Compare July 15, 2021 19:52
@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #15719 (4726481) into master (fe68e45) will decrease coverage by 0.22%.
The diff coverage is 50.00%.

❗ Current head 4726481 differs from pull request most recent head 20c9c01. Consider uploading reports for the commit 20c9c01 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15719      +/-   ##
==========================================
- Coverage   76.91%   76.69%   -0.23%     
==========================================
  Files         983      983              
  Lines       51609    51607       -2     
  Branches     6988     6988              
==========================================
- Hits        39696    39580     -116     
- Misses      11690    11804     +114     
  Partials      223      223              
Flag Coverage Δ
hive ?
mysql 81.55% <50.00%> (+<0.01%) ⬆️
postgres 81.57% <50.00%> (+<0.01%) ⬆️
presto ?
python 81.66% <50.00%> (-0.43%) ⬇️
sqlite 81.18% <50.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/utils/mock_data.py 25.18% <50.00%> (+0.36%) ⬆️
superset/db_engines/hive.py 0.00% <0.00%> (-82.15%) ⬇️
superset/db_engine_specs/hive.py 69.44% <0.00%> (-17.07%) ⬇️
superset/db_engine_specs/presto.py 83.36% <0.00%> (-6.53%) ⬇️
superset/views/database/mixins.py 81.03% <0.00%> (-1.73%) ⬇️
superset/connectors/sqla/models.py 88.26% <0.00%> (-1.65%) ⬇️
superset/db_engine_specs/base.py 88.28% <0.00%> (-0.40%) ⬇️
superset/models/core.py 89.79% <0.00%> (-0.27%) ⬇️
superset/utils/core.py 88.12% <0.00%> (-0.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe68e45...20c9c01. Read the comment docs.

scripts/benchmark_migration.py Outdated Show resolved Hide resolved
requirements/development.in Outdated Show resolved Hide resolved
superset/utils/mock_data.py Outdated Show resolved Hide resolved
"""
Add entities of a given model.

Copy link
Member

Choose a reason for hiding this comment

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

Keep this empty line.

@@ -69,6 +69,9 @@

# pylint: disable=too-many-return-statements, too-many-branches
def get_type_generator(sqltype: sqlalchemy.sql.sqltypes) -> Callable[[], Any]:
if isinstance(sqltype, sqlalchemy.dialects.mysql.types.TINYINT):
Copy link
Member

Choose a reason for hiding this comment

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

These changes were merged already, right?

start = time.time()
bar.finish()
Copy link
Member

@betodealmeida betodealmeida Jul 16, 2021

Choose a reason for hiding this comment

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

So, there's a few subtle bugs in your implementation. The critical one is that for every model you define entities as an empty list. But you only add the entities to the session after you've processed all models (line 198). This means you'll only add the entities of the last model in the for model in models loop.

I'd also only show the progress bar if you have something to add.

Something like this:

while min_entities <= limit:
    downgrade(revision=down_revision)
    print(f"Running with at least {min_entities} entities of each model")
    for model in models:
        missing = min_entities - model_rows[model]
        if missing > 0:
            entities: List[Model] = []
            bar = ChargingBar(f"Adding {missing} entities to the {model.__name__} model", max=missing)
            try:
                for entity in add_sample_rows(session, model, missing)
                    entities.append(entity)
                    bar.next()
            except Exception:
                session.rollback()
                raise
            bar.finish()
            model_rows[model] = min_entities

            # add all created entities
            session.add_all(entities)
            session.commit()

            if auto_cleanup:
                new_models[model].extend(entities)

@AAfghahi AAfghahi requested a review from a team as a code owner July 16, 2021 14:25
@AAfghahi AAfghahi force-pushed the arash/progressBar branch 3 times, most recently from d63372e to 4f97f6d Compare July 16, 2021 14:42
@@ -177,18 +178,23 @@ def main(
for model in models:
missing = min_entities - model_rows[model]
if missing > 0:
entities: List[Model] = []
bar = ChargingBar("Processing", max=missing)
Copy link
Member

Choose a reason for hiding this comment

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

How does the print in 183 interact with the bar? I think it might be better to reverse the order of 182,183.

Can you post a screencast of the progress bar in action? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I just switched and pushed up, it does look a lot better. Putting a screencast in the main portion.

Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

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

So cool!

@betodealmeida betodealmeida merged commit 80dd525 into apache:master Jul 19, 2021
lyndsiWilliams pushed a commit to preset-io/superset that referenced this pull request Jul 26, 2021
* rough draft of benchmark script

* revisions

* revisions

* rough draft of benchmark script

* revisions

* Update requirements/development.in

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/utils/mock_data.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* more revisions

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
eschutho pushed a commit that referenced this pull request Jul 28, 2021
#15856)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>

* feat: toggle the ReportModal Icon based on user permissions (#15780)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* added testing props

* cleaned up rebase

* changed name and type

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* feat: Begin incorporation of email report modal to Charts (#15793)

* Add email report modal to Charts

* Fix px themes

* feat: fetch UI specific reports  (#15785)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* created Api call

* added click logic

* made the fetch report into a action/reducer

* abstracted report action

* revision

* added reportState to reducer

* reports conditions

* revisions

* revisions

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* fix: Edit physical dataset from the Edit Dataset modal (#15770)

* Remove unnecessary onChange

* Remove confliciting onChange

* Revert unnecessary change

* Enhance and fix tests

* feat: add show columns to Reports model (#15712)

* added logic for creation_method

* revisions

* added index

* Update superset/migrations/versions/3317e9248280_add_creation_method_to_reports_model.py

* filters

* search columns updated

* fix: margin right on warning icon to 8px (#15715)

* changed margin right on warning icon to 8px

* fixed to grid units from pixels

* feat: adding Progress Bar to Benchmark script (#15719)

* rough draft of benchmark script

* revisions

* revisions

* rough draft of benchmark script

* revisions

* Update requirements/development.in

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/utils/mock_data.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* more revisions

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* fix: migration downgrade references wrong column (#15791)

* fix(dashboard): Add resize handles to right and bottom of component (#15778)

* fix(dashboard): Add resize handles to right and bottom of component

* Fix test

* chore: Add tags to the new viz gallery (#15734)

* chore: add tags to gallery

* fix UT

* fix lint

* redesign tags filter

* chore: change to Highly-used and fix some css

* fix UT

Co-authored-by: stephenLYZ <750188453@qq.com>

* chore: bump 0.17.70 (#15795)

* docs: Adding Sunbird to users list (#15794)

* chore: Improves the Select component UI/UX - iteration 4 (#15480)

* Add z-index only on maximize (#15800)

* chore: remove `retry` dependency in favor of `backoff` (#15788)

* chore: remove retry dep in favor of backoff

* Fix lint

* fix: create fk model in benchmark script (#15804)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* Working on onSave functionality

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>

* feat: toggle the ReportModal Icon based on user permissions (#15780)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* added testing props

* cleaned up rebase

* changed name and type

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* feat: fetch UI specific reports  (#15785)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* created Api call

* added click logic

* made the fetch report into a action/reducer

* abstracted report action

* revision

* added reportState to reducer

* reports conditions

* revisions

* revisions

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* Dashboard onSave progress

* More onSave/create progress

* Dashboard POST working!

* Dashboard POST relocated to redux

* POST now also working in Charts

* making linter happy

* linter again

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Co-authored-by: stellalc7 <stellalc7@gmail.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Yongjie Zhao <yongjie.zhao@gmail.com>
Co-authored-by: stephenLYZ <750188453@qq.com>
Co-authored-by: Kumar <kumarks1122@gmail.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
eschutho pushed a commit that referenced this pull request Jul 30, 2021
#15856)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>

* feat: toggle the ReportModal Icon based on user permissions (#15780)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* added testing props

* cleaned up rebase

* changed name and type

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* feat: Begin incorporation of email report modal to Charts (#15793)

* Add email report modal to Charts

* Fix px themes

* feat: fetch UI specific reports  (#15785)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* created Api call

* added click logic

* made the fetch report into a action/reducer

* abstracted report action

* revision

* added reportState to reducer

* reports conditions

* revisions

* revisions

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* fix: Edit physical dataset from the Edit Dataset modal (#15770)

* Remove unnecessary onChange

* Remove confliciting onChange

* Revert unnecessary change

* Enhance and fix tests

* feat: add show columns to Reports model (#15712)

* added logic for creation_method

* revisions

* added index

* Update superset/migrations/versions/3317e9248280_add_creation_method_to_reports_model.py

* filters

* search columns updated

* fix: margin right on warning icon to 8px (#15715)

* changed margin right on warning icon to 8px

* fixed to grid units from pixels

* feat: adding Progress Bar to Benchmark script (#15719)

* rough draft of benchmark script

* revisions

* revisions

* rough draft of benchmark script

* revisions

* Update requirements/development.in

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/utils/mock_data.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* more revisions

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* fix: migration downgrade references wrong column (#15791)

* fix(dashboard): Add resize handles to right and bottom of component (#15778)

* fix(dashboard): Add resize handles to right and bottom of component

* Fix test

* chore: Add tags to the new viz gallery (#15734)

* chore: add tags to gallery

* fix UT

* fix lint

* redesign tags filter

* chore: change to Highly-used and fix some css

* fix UT

Co-authored-by: stephenLYZ <750188453@qq.com>

* chore: bump 0.17.70 (#15795)

* docs: Adding Sunbird to users list (#15794)

* chore: Improves the Select component UI/UX - iteration 4 (#15480)

* Add z-index only on maximize (#15800)

* chore: remove `retry` dependency in favor of `backoff` (#15788)

* chore: remove retry dep in favor of backoff

* Fix lint

* fix: create fk model in benchmark script (#15804)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* Working on onSave functionality

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>

* feat: toggle the ReportModal Icon based on user permissions (#15780)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* added testing props

* cleaned up rebase

* changed name and type

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* feat: fetch UI specific reports  (#15785)

* style: Rough draft of email report modal (#15666)

* clears errors when closing out of modal (#15623)

* fix: avoid fetching favorite status for anonymous user (#15590)

* avoid fetching favorite status for anonymous user

* add test + fix types

* fix lint errors

* Building ReportModal component

* Continued ReportModal creation

* Visual details updated

* CronError style

* Very basic testing added

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* first draft

* created Api call

* added click logic

* made the fetch report into a action/reducer

* abstracted report action

* revision

* added reportState to reducer

* reports conditions

* revisions

* revisions

Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>

* Dashboard onSave progress

* More onSave/create progress

* Dashboard POST working!

* Dashboard POST relocated to redux

* POST now also working in Charts

* making linter happy

* linter again

Co-authored-by: AAfghahi <48933336+AAfghahi@users.noreply.github.com>
Co-authored-by: aspedrosa <aspedrosa@ua.pt>
Co-authored-by: Geido <60598000+geido@users.noreply.github.com>
Co-authored-by: stellalc7 <stellalc7@gmail.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com>
Co-authored-by: Yongjie Zhao <yongjie.zhao@gmail.com>
Co-authored-by: stephenLYZ <750188453@qq.com>
Co-authored-by: Kumar <kumarks1122@gmail.com>
Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com>
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
* rough draft of benchmark script

* revisions

* revisions

* rough draft of benchmark script

* revisions

* Update requirements/development.in

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/utils/mock_data.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* more revisions

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
* rough draft of benchmark script

* revisions

* revisions

* rough draft of benchmark script

* revisions

* Update requirements/development.in

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/utils/mock_data.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* more revisions

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* rough draft of benchmark script

* revisions

* revisions

* rough draft of benchmark script

* revisions

* Update requirements/development.in

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* Update superset/utils/mock_data.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* more revisions

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants