-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
chore: update docs for new alerts and reporting feature #13104
Conversation
fix bullets fix redis depends on name in docker-compose template
FYI, I have just sent PR #13116 which would enable this on Kubernetes by (optionally) running Celery beat as a separate singleton pod. The rest of the configuration can be injected using the existing chart mechanism:
|
step added: Upgrade the db step added: init step added: setup admin fix redis superset naming add . to docker build command
change postgres to internal so it persists on docker-compose down
@@ -6,8 +6,309 @@ index: 10 | |||
version: 1 | |||
--- | |||
|
|||
## Scheduling and Emailing Reports | |||
## Alerts and Reports |
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.
I'm not really comfy about putting the Alerts & Reports doc in a page named email-reports
, as this doc concern alerts too, and not only emails but also Slack 🤷
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.
I'd definitely agree, was really just for organisation so that in the menu there isn't 'Alerts and reporting' and then below it 'Scheduling and Emailing reports' which would be confusing imo, but if @srinify can help we could put it into its own section 'Alerts and Reporting', and then have the legacy reporting stuff remain at the bottom of this file?
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.
I agree, renaming and keep the old approach at the end seems the best!
@leocape this is an excellent community effort in documenting a highly requested and very confusing feature set. I'm wondering how much of the config changes can be included in the default superset files ( |
I also opened #13143 which includes a new docker-compose file for non-dev use cases. I added a beat container which I realized was missing in the regular docker-compose workflow. |
Thanks @nytai with all the help from everyone on the Slack threads. Yes would be great to simplify this doc so that it focuses more/only on the feature related config. I could cut out the superfluous docker-compose and dockerfile if you can include the webdriver (and postgres?) into the default dockerfile? |
Thanks @Yann-OAF I could link to it from this doc where I've placed the link placeholder? |
mv chromedriver /usr/bin && \ | ||
apt autoremove -yqq --purge && \ | ||
apt clean && \ | ||
rm -f google-chrome-stable_current_amd64.deb chromedriver_linux64.zip |
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.
I'm curious, would you have similar working instructions to install geckodriver
? My attempts have been unsuccessful...
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.
I couldn't get it working with geckodriver/firefox, no matter which version I tried, it wouldn't launch correctly when trying to take the screenshot... I suspect there is a missing config somewhere that is needed for it
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.
It works with geckodriver
too, we can adapt the doc to propose both approaches.
Yes, sounds good. These are probably too much to integrate to this doc, and won't be of interest to everyone (only k8s users) so linking is probably enough! I'm thinking I might start a dedicated doc on running in Kubernetes |
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.
@leocape this is an excellent community effort in documenting a highly requested and very confusing feature set. I'm wondering how much of the config changes can be included in the default superset files (
superset/config.py
,Dockerfile
,docker-compose.yml
), so only minimal changes are required to enable this feature for testing. Documentation can then focus more on how to deploy this in prod-like environments (ie, not using docker-compose). Thoughts @dpgaspar? Are there any blockers (licensing) to adding the browser + webdriver to the Dockerfile?
I think it does not make sense to release a dockerfile with an webdriver + browser installed, and that's where we need to exclude all non compliant ASF packages/software. So IMO we should keep the docker file lean and clean, and add hooks so that it's easy to add additional packages or software (on this case)
btw, chrome driver license is 3-BSD, so it's "compatible" with ASF.
RUN apt update | ||
RUN wget https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb && \ | ||
apt install -y --no-install-recommends ./google-chrome-stable_current_amd64.deb && \ | ||
wget https://chromedriver.storage.googleapis.com/88.0.4324.96/chromedriver_linux64.zip && \ |
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.
These two versions will eventually differ
Ok, in that case we should keep the Dockerfile changes. I'm planning to add the beat container in #13143 so that should take care of some the docker-compose changes, though it will need to be updated to reference the new images. |
4. Create a new sub directory called `config` | ||
5. Create the `superset_config.py` file in the `config` sub directory | ||
6. Run the image using `docker-compose up` in the same directory as the `docker-compose.py` file | ||
7. In a new terminal window, upgrade the DB by running `docker exec -it superset-1.0.1-extended superset db upgrade` |
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.
is there a better way to run
superset db upgrade
superset init
superset fab create-admin
as part of startup without entering the container?
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.
i think the superset-init
service in the docker-compose workflow should take care of this.
Hey @vnourdin @leocape I'm merging this as-is for now! The clean up process is quite involved, I like some things about the older commit and the latest commit and there's a lot of context to resolve. For now, it'd be nice to just have the info out there. So I'm merging for now, and I'll do a second pass next week to clean up and organize better! |
and after done all, what`s the account and password? I tried admin/admin, wrong |
* update docs for new alerts and reporting feature * fix punctuation * fix punctuation * fix punctuation fix bullets fix redis depends on name in docker-compose template * fix bullets * Add extra steps for getting v1.0.1 running step added: Upgrade the db step added: init step added: setup admin fix redis superset naming add . to docker build command * fix worker to use gevent instead of prefork change postgres to internal so it persists on docker-compose down * add comments to superset_config.py * ref: Re-organize a bit and make it easier to read * ref: rm useless level * docs: md changes + more details + kubernetes * docs: Rename to Alerts and Reports * docs: Link to Kubernetes doc * docs: details for docker-compose + minor md changes * docs: fix lists indent * docs: remove unused config * docs: Put detailed config and dockerfile before specific stuff * docs: clean Dockerfiles Co-authored-by: Valentin NOURDIN <vnourdin.externe@bedrockstreaming.com>
SUMMARY
Added docs for setting up and configuring the new alerts and reports in V.1.0
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
Using the templates outlined in the doc, run these steps to get the new alerts and reporting feature working. After these steps are completed, you should now be able to create and test alerts and reports
docker-compose.yaml
file in the same directoryconfig
superset_config.py
file in theconfig
sub directorydocker-compose up
in the same directory as thedocker-compose.py
fileADDITIONAL INFORMATION