ohad24 / encyclo-flower Goto Github PK

Our swag gateway doesn't have a dev version so this is temporary, latest tag will be applied soon after some testing

Originally posted by @ofersadan85 in #53 (comment)

Hi,
Based on

@ofersadan85, need you to investigate please

Thanks

This error prevents any tests from Dependabot to pass (Dependabot can't access secrets)
I can try to fix this by splitting Dependabot workflows from the others (Will improve CI)
let me know if you want me to work on this

The error in Dependabot PR:

Error: google-github-actions/auth failed with: the GitHub Action workflow must specify exactly one of "workload_identity_provider" or "credentials_json"! If you are specifying input values via GitHub secrets, ensure the secret is being injected into the environment. By default, secrets are not passed to workflows triggered from forks, including Dependabot.

After merging to master, the API isn't updated correctly on the server.

https://github.com/ohad24/encyclo-flower/runs/6142171221?check_suite_focus=true#step:3:28

Because no cred supply

rewrite content and order

Based on #110
We need a visualization and notifications based on the metrics

Production server and dev

Done in fbdc3a0 But i still recommend to open an issue about it later

Originally posted by @ofersadan85 in #32 (comment)

from https://about.codecov.io/ with a badge. after committing to main (or from PR merge)

Rewrite file for more clarity

Refactor API and switch from datetime.utcnow() to datetime.now(), and check docker-compose file is updated with the TZ environment variable.

API should fail gracefully in any of these conditions:

• MONGO_URI is not set or the connection fails
• GOOGLE_APPLICATION_CREDENTIALS is not set or the file doesn't exist or the storage itself isn't available

To fail gracefully is to continue working but to return error messages as replies to API calls

We've discussed it a lot, the issue with google storage is already being worked on in #17 in tests (but should also not be an issue in a working API instance without tests).

I suggest to keep this issue open to track this and as a reminder, but it's not urgent

This is for several reasons that aren't related to dependabot as in #16 or pytest in #17 :

• Docker image creation fails without a clear error indicating what the problem is, hard to debug
• Test/dev environments are more difficult to setup quickly (even if unrelated to running tests)
• At the very minimum, a "hello world" function that doesn't require extra resources shouldn't fail when un-needed resources aren't available, this will be easier to scale-up in the future (if the API is split between servers with different resources, for example)

reference: https://github.com/ohad24/encyclo-flower/runs/6879276899?check_suite_focus=true

while building / testing
and badge

The reason why we have so many package conflicts (as shown by dependabot pull requests) is beacuse the requirements.txt is too specific about packages we don't care about (because of pip freeze)

src/api/requirements.txt should only contain packages that we explicitly need for the api and pip should resolve the dependencies automatically. We will receive less dependabot pull requests and all of them will be 100% relevant. Another advantage is that no "dev" packages (black for example) are needed for the api docker image. If we need "dev" packages, they should be in another requirements.txt in the root of the repository

If it seems ok to you I suggest I will minimize src/api/requirements.txt as much as possible (the minimal version that passes all tests)

1. This script loads to a local mongodb and also to an external mongodb URI, it should instead only load to the URI (so we can decide if it's only local or not based on the environment variable).
2. scripts/plant_data_17_02_2022.json is hardcoded and will need to be changed if test data changes, it's better to setup loading of all *.json files in this folder

Tracking issue for:

• https://github.com/ohad24/encyclo-flower/security/code-scanning/67

This comment should be removed

When recreating the server the latest problem was that metadata wasn't being fetched because of missing dependency jq - it must be added to the required apt packages in the ansible playbook

https://lgtm.com/

Using this to track problems from #61

Just tracking a bug I found while working on #34

Caused by: 6bc0673

Fix probably in src/api/Dockerfile

See #FIXME comments: 8254e35

There are several dependencies in src/api/requirements.txt that are conflicting with one another

To replicate, run this on a new clone of the repository:

git clone [email protected]:ohad24/encyclo-flower.git
cd encyclo-flower
python3.10 -m venv venv
source venv/bin/activate
pip install --upgrade -r src/api/requirements.txt

Example error 1:

ERROR: Cannot install -r src/api/requirements.txt (line 25) and cachetools==5.0.0 because these package versions have conflicting dependencies.

The conflict is caused by:
The user requested cachetools==5.0.0
google-auth 2.3.3 depends on cachetools<5.0 and >=2.0.0

I fixed the above by changing google-auth==2.3.3 to google-auth>=2.3.3 (which installed version 2.6.0)
I fixed a similar issue with black==21.9b0 by installing black==22.1.0
After those 2 fixes the install went off perfectly.

Suggestion 1: Update versions in src/api/requirements.txt
Suggestion 2: Set up automatic pull request for newer package versions - this will also fix security issues with older packages that are not being updated regularly

Let me know if you want me to work on one of these suggestions

Codacy detected an issue:

Message: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

Currently on:

• Commit: cf1cf17
• File: server/deploy_ansible.sh
• LineNum: 1

The exporter
https://github.com/trallnag/prometheus-fastapi-instrumentator

I can't test it yet but the easiest approach would be something like this:

def prepare_search_query(search_input) -> dict:
    query_and = []
    for attribute in dir(search_input):
        value = getattr(search_input, attribute, None)
        if attribute.startswith('_') or value is None:
            continue
        elif attribute == 'name_text':
            query_and.append(prepare_query_plant_name_text(search_input.name_text))
        elif attribute == 'location_names':
            for location_name in search_input.location_names:
                query_and.append({"locations.location_name": location_name})
        elif isinstance(value, bool):
            query_and.append({attribute: value})
        elif isinstance(value, str):
            query_and.append({attribute: {"$in": value}})

    if not query_and:
        raise HTTPException(
            status_code=400,
            detail="must supply at least one parameter",
        )
    query = {"$and": query_and}
    return query

However this can also be improved in the 'search_input' object itself, but I was unable to locate where it's coming from or where it's class is defined

https://github.com/ohad24/encyclo-flower/runs/5855649580?check_suite_focus=true#step:4:7

Maybe because I let him (the bot) to merge (using @dependabot merge) ?

@ofersadan85, please advise.

Thanks