feat: Adds the /explore endpoint to the v1 API

[michael-s-molina]

SUMMARY

This PR is part of the Single Page App initiative and it adds the /explore endpoint to the v1 API. The previous legacy endpoint in superset.views.core was responsible not only for assembling the Explore context but also to save Explore-related information. This endpoint, however, does not contain any save operation. We should use the existing v1 endpoints to save the charts and dashboard relationships. Another important difference is that this endpoint does not return shared bootstrap data such as user, permissions, common, etc. as these are already available by other means.

The legacy endpoint also contained redirects and flash messages. These were transformed into HTTP response errors/values and should be handled by the client-side.

Much of the code in the new endpoint comes from the legacy one. It's important to mention that it is not the objective of the PR to refactor the code at this point to avoid introducing potential regressions.

GET /v1/explore/

QUERY PARAMS # all optional
   form_data_key=<key>
   permalink_key=<key>
   slice_id=<id>
   dataset_id=<id>
   dataset_type=<type> 

RESPONSE 
{ 
   form_data: <object>,
   dataset: <object>,
   slice: <object>,
   message: <string> # information related to the request
}

We will remove the legacy endpoint after the SPA initiative is completed.

TESTING INSTRUCTIONS

1 - Execute the API tests
2 - All tests should pass

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• 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

[codecov]

Codecov Report

Merging #20399 (e4161f0) into master (cf4f05e) will increase coverage by 0.11%.
The diff coverage is 92.79%.

@@            Coverage Diff             @@
##           master   #20399      +/-   ##
==========================================
+ Coverage   66.72%   66.83%   +0.11%     
==========================================
  Files        1739     1744       +5     
  Lines       65130    65380     +250     
  Branches     6898     6898              
==========================================
+ Hits        43459    43698     +239     
- Misses      19921    19932      +11     
  Partials     1750     1750              

Flag	Coverage Δ
hive	53.76% <59.45%> (+0.02%)	⬆️
mysql	82.41% <92.79%> (?)
postgres	82.46% <92.79%> (+0.06%)	⬆️
presto	53.62% <59.45%> (+0.02%)	⬆️
python	82.90% <92.79%> (+0.10%)	⬆️
sqlite	82.26% <92.79%> (+0.07%)	⬆️
unit	50.62% <59.45%> (+0.04%)	⬆️

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

Impacted Files	Coverage Δ
superset/explore/api.py	86.36% <86.36%> (ø)
superset/explore/commands/get.py	89.24% <89.24%> (ø)
superset/explore/commands/parameters.py	100.00% <100.00%> (ø)
superset/explore/exceptions.py	100.00% <100.00%> (ø)
superset/explore/form_data/api.py	95.69% <100.00%> (-0.05%)	⬇️
superset/explore/schemas.py	100.00% <100.00%> (ø)
superset/initialization/__init__.py	91.57% <100.00%> (+0.05%)	⬆️
superset/views/core.py	77.38% <100.00%> (+0.49%)	⬆️
superset/reports/commands/log_prune.py	85.71% <0.00%> (-3.58%)	⬇️
superset/commands/importers/v1/utils.py	92.20% <0.00%> (-1.30%)	⬇️
... and 10 more

---

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 cf4f05e...e4161f0. Read the comment docs.

[zhaoyongjie]

Thanks for improving explore endpoint, I have a first quick review and leave some comments.

[kgabryje]

LGTM!

[dpgaspar]

Looking good!

Can you add unittests for GetExploreCommand also

[kgabryje]

CC @eric-briscoe since you were interested in this project 🙂

[michael-s-molina]

Thanks for your review @dpgaspar. I really appreciate it!

I noticed that many concepts are shared among API schema definitions (database, form_data, dataset, etc). Currently, some are duplicated in each endpoint and are inconsistent with one another. One thing we could do in a follow-up is to create many of the schema concepts in a more global way, to share both structure and documentation. We should also provide the ability to export a concept partially like just exposing some specific properties of the global database definition. This would improve our API standardization a lot not only by ensuring an object is correctly defined but also that the documentation associated with that object is consistent among endpoints.

[michael-s-molina]

Can you add unittests for GetExploreCommand also

Since the API is pretty much invoking the GetExploreCommand, with no extra logic, the tests for the command would be very similar. So I chose to just test the API layer because it seemed like the best cost/benefit approach.

[dpgaspar]

Thanks for your review @dpgaspar. I really appreciate it!

I noticed that many concepts are shared among API schema definitions (database, form_data, dataset, etc). Currently, some are duplicated in each endpoint and are inconsistent with one another. One thing we could do in a follow-up is to create many of the schema concepts in a more global way, to share both structure and documentation. We should also provide the ability to export a concept partially like just exposing some specific properties of the global database definition. This would improve our API standardization a lot not only by ensuring an object is correctly defined but also that the documentation associated with that object is consistent among endpoints.

sounds promising!

[dpgaspar]

Can you add unittests for GetExploreCommand also

Since the API is pretty much invoking the GetExploreCommand, with no extra logic, the tests for the command would be very similar. So I chose to just test the API layer because it seemed like the best cost/benefit approach.

I understand, my current concern is that the defined response payload is complex and I don't see them been asserted on the API tests, for example dataset and slice are always asserted has None. Can you add tests for them?

[dpgaspar]

btw, we are currently adding this: https://github.com/apache/superset/blob/master/superset/views/core.py#L1352
to the migrated /superset/* endpoints, not actually deleting them, the reason being that actually deleting the endpoints is considered a breaking change. The plan is to delete all these on v3

[michael-s-molina]

btw, we are currently adding this: https://github.com/apache/superset/blob/master/superset/views/core.py#L1352 to the migrated /superset/* endpoints, not actually deleting them, the reason being that actually deleting the endpoints is considered a breaking change. The plan is to delete all these on v3

Nice! I'll add this to the old explore endpoint as well.

[michael-s-molina]

I understand, my current concern is that the defined response payload is complex and I don't see them been asserted on the API tests, for example dataset and slice are always asserted has None. Can you add tests for them?

@dpgaspar I added the assertions to dataset and slice. Since they are huge trees, I partially matched using the most important attributes.

I also added the warning to the old explore endpoint.