Initial app-canary

[cgraham-rs]

Screenshot from deployment to Connect

When the required Var is not set

Email report when any content has failed.

[jonkeane]

Would you mind trying to remove this and use Publisher's scan instead? I suspect that will make a much more limited requirements.txt

Also, we should consider not pinning every single dependency here. Floating versions so long as they are compatible will mean less maintenance burden over time.

[jonkeane]

A bit more context on why requirements.txt generated by rsconnect-python aren't necessarily what we want: posit-dev/rsconnect-python#538

[cgraham-rs]

Would you mind trying to remove this and use Publisher's scan instead? I suspect that will make a much more limited requirements.txt

I'm definitely having issues with rsconnect-python with this extension. It does not seem to be scanning the qmd for requirements and building a requirements.txt with just the used packages like I believe it should. Most likely user error as it does not appear I had this same issue with other Python extensions.

Also, we should consider not pinning every single dependency here. Floating versions so long as they are compatible will mean less maintenance burden over time.

This probably requires more discussion and education on my end. Both rsconnect and Publisher scan currently create tightly pinned requirements.txt with no obvious way to control that. And all of the current extensions in the repo appear to have this same concern.

[jonkeane]

This probably requires more discussion and education on my end. Both rsconnect and Publisher scan currently create tightly pinned requirements.txt with no obvious way to control that. And all of the current extensions in the repo appear to have this same concern.

Yeah, this has been a common pattern, but one we need to step away from. Pinning everything to exactly one version means that if any of these have upgrades with security fixes we will need to issue a new release bumping the pin. If we have something like >x.y.z or no version specification at all, folks will get fixed versions automatically without us doing anything.

[jonkeane]

Would you mind doing this, please?

[cgraham-rs]

Would you mind doing this, please?

Done.

[toph-allen]

This isn't a full review, since this is a draft.

I don't actually know how Quarto dashboards work! Do they run in-browser via web assembly or use some other framework to run? My initial thought was, "maybe there's a better framework", but actually I think it might be a great choice — good to cover different types of content that Connect supports!

Setting the GUIDs to track via an environment variable kind of reveals the lack of mutable local storage for content, but it seems like a good solution! Plus, in a future iteration, you could build UI to manage GUIDs by settings its environment variable via the SDK.

[toph-allen]

Oops, I didn't mean to hit "approve", meant to hit "comment".

[cgraham-rs]

I don't actually know how Quarto dashboards work! Do they run in-browser via web assembly or use some other framework to run? My initial thought was, "maybe there's a better framework", but actually I think it might be a great choice — good to cover different types of content that Connect supports!

Quarto was chosen so we can leverage the Connect scheduler feature, which as one of the requirements.

Setting the GUIDs to track via an environment variable kind of reveals the lack of mutable local storage for content, but it seems like a good solution! Plus, in a future iteration, you could build UI to manage GUIDs by settings its environment variable via the SDK.

Agreed, my assumption is a UI-based way to manage the GUID is likely going to be the first thing a user would request. It's not a good experiencing doing it manually via env vars.

[jonkeane]

I don't actually know how Quarto dashboards work! Do they run in-browser via web assembly or use some other framework to run? My initial thought was, "maybe there's a better framework", but actually I think it might be a great choice — good to cover different types of content that Connect supports!

Quarto was chosen so we can leverage the Connect scheduler feature, which as one of the requirements.

Spot on, @cgraham-rs for a bit more context: this looks like a standard (rendered) quarto document / "report" what you have here (which is great, possibly even just fine / exactly what we need!) there is a thing known as a "Quarto Dashboard" which is very similar to a rendered quarto document, but presents information with cards in a view that is much more reminiscent of a dashboard. But under the hood these are all rendered types on Connect, where connect does ~ quarto render and then it displays the output of that. There isn't any magical WASM or anything like that here (though, technically, one could add a WASM chunk to a quarto doc and then it would be that!)

[cgraham-rs]

Shall we review and merge this so we can enjoy tiny follow-on PR reviews?

[marcosnav]

I'm not a Quarto expert and I might be missing conventions, but here I have some suggestions:

There are many if blocks to add env vars instructions when needed, I think you could have a method looping through the environment vars required, re-structuring these a bit e.g:

required_env_vars = ["CONNECT_SERVER", "CONNECT_API_KEY", "CANARY_GUIDS"]

def check_required_env():
    env_values = []
    for reqenv in required_env_vars:
        env_value = os.environ.get(reqenv, "")
        if not env_value:
            show_instructions = True
            instructions.append(f"Please set the {reqenv} environment variable.")
        else:
            env_vars.append(env_value)
    return env_values

def unpack_guids(guids_str):
    guids = [guid.strip() for guid in app_guid_str.split(',') if guid.strip()]
    if not guids:
        show_instructions = True
        instructions.append("CANARY_GUIDS should be a comma separated list with content GUIDs you wish to monitor.")
    return guids

connect_server, api_key, app_guids_str = check_required_env()
app_guids = unpack_guids(app_guids_str)

if show_instructions:
    # We'll use this flag later to display instructions instead of results
    results = []
    df = pd.DataFrame()  # Empty DataFrame
    check_time = datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S')
else:
    prep_apps_status()

Notice above in my suggestion the prep_apps_status() func, I'm not a fan of having big chunks of code within if|else blocks, so many things that you have within the else block when all env vars are ok, can be taken out. e.g:

def server_check():
# Headers for Connect API
    headers = {"Authorization": f"Key {api_key}"}
    # Check if server is reachable
    try:
        server_check = requests.get(
            f"{connect_server}/__ping__", 
            headers=headers, 
            timeout=5
        )
        server_check.raise_for_status()
    except requests.exceptions.RequestException as e:
        raise RuntimeError(f"Connect server at {connect_server} is unavailable: {str(e)}")

def get_app_details():
    ...

def validate_app():
    ...

def prep_apps_status():
    server_check()

    # Check all apps and collect results
    results = []
    for guid in app_guids:
        results.append(validate_app(guid))
    ...

Similarly, for the second python block, you could have methods like render_instructions() and render_apps_table().

In general, my suggestion would be to leverage a bit more on functions to have smaller units of code responsible of one thing at a time, splitting in functions will also help in the future to maintain or update as needed by quickly noticing which function is responsible of xyz.

[toph-allen]

I'm going to approve this version — I think this is a good iteration, and worth gathering feedback on, so all of my comments, take as feedback to fix in future versions.

• Definitely some rough edges around setup, and I think perhaps those should be the focus for improvements? Definitely at a point where it's worth gathering feedback, because maybe those issues are big enough to cause a change in direction or priorities?
• I deployed it and set it up, and the email worked.
• In the table, probably not necessary to have the content guid be a link that goes to the same place as the content title. (makes it harder to highlight and copy too)

Good first iteration!

[toph-allen]

Perhaps this isn't the best text to display on-screen — "setup has failed" makes it sound like something has gone wrong, when actually, the user just has to take a few more actions to finish setting the app up.

We might want a more detailed step by step set of instructions in a final version, but not necessary in this iteration of course.

[cgraham-rs]

I think all of the comments related to the setup challenges should be rectified by implementing GUID management directly in the report vs in the Connect vars UI.

[toph-allen]

Nice :)

[toph-allen]

The instructions should also include a prominent step which is "Rerun the report" after adding the variable. I added the environment variable and thought the app was broken because it didn't automatically rerender, and it was only my Connect dev knowledge of "Ohhh, maybe it didn't automatically rerun" which helped me figure out that I needed to rerun it.

[toph-allen]

Perhaps also worth having an instruction to set it to run on a schedule, as that's the whole reason it's a report, right?

[toph-allen]

Also — I wonder if it's worth, in a future edition, having some buttons which actually use the Connect API via the posit-sdk to adjust the environment variables at the user's request. It might also be nice to have a "refresh" button on the report itself!

[cgraham-rs]

Automatically re-rendering after updating the GUIDs sounds like a good idea, added to #168

[cgraham-rs]

Thanks for everyone's comments. The various feedback has been captured in new issues and will be implemented in incremental follow-on PRs.