Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #218 +/- ##
===========================================
+ Coverage 73.21% 73.34% +0.13%
===========================================
Files 363 363
Lines 21968 21968
Branches 2258 2258
===========================================
+ Hits 16083 16113 +30
+ Misses 5885 5855 -30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tbkr
left a comment
tbkr
left a comment
There was a problem hiding this comment.
LGTM, left some small remarks.
| env: | ||
| REGISTRY: eccr.ecmwf.int | ||
| IMAGE_NAME: z3fdb-web-service | ||
| PROJECT_NAME: kkratz |
There was a problem hiding this comment.
Should this still be in this path or do we have a dmst project name?
There was a problem hiding this comment.
This will change later when we the service gets integrated.
| uses: docker/login-action@v3 | ||
| with: | ||
| registry: ${{ env.REGISTRY }} | ||
| username: ${{ secrets.KKRATZ_ECCR_USER }} |
There was a problem hiding this comment.
Those should probably also be the DMST/FDB_ECCR credentials?
There was a problem hiding this comment.
No, this will change later when the service gets integrated.
| -DCMAKE_PREFIX_PATH=dependencies \ | ||
| -DCMAKE_BUILD_TYPE=RelWithDebInfo | ||
| -DCMAKE_BUILD_TYPE=RelWithDebInfo \ | ||
| -DENABLE_MARS2GRIB_PYTHON=OFF |
There was a problem hiding this comment.
Could you add a command on why this has been disabled. Mention the breakage and a potential remove of the flag once this has been resolved?
Maybe we should address whether this should be enabled per default?
| image: {{ .Values.image.z3fdb.repository }}:{{ .Values.image.z3fdb.tag }} | ||
| args: ["--fdb-config=/etc/fdb/fdb.config"] | ||
| env: | ||
| - name: FDB_DEBUG |
There was a problem hiding this comment.
This debug flag shouldn't be enabled per default.
| metadata: | ||
| name: z3fdb-service | ||
| spec: | ||
| type: NodePort # Or NodePort for simpler Minikube access |
There was a problem hiding this comment.
This comment can also be removed.
| cd "$src" | ||
| link_dirs "$root" "${required_repos[@]}" | ||
| cat << 'EOF' > CMakeLists.txt | ||
| cmake_minimum_required(VERSION 3.12 FATAL_ERROR) |
There was a problem hiding this comment.
Don't we rely on cmake >4.0 internally? Should this be adjusted?
tbkr
left a comment
tbkr
left a comment
There was a problem hiding this comment.
Could you have a look at the failing CI?
Ozaq
force-pushed
the
z3fdb/climate-dt-on-lumi-databridge
branch
2 times, most recently
from
5814059 to
da7b7e2
Compare
Co-authored-by: Tobias Kremer <tobias.kremer@ecmwf.int>
Ozaq
force-pushed
the
z3fdb/climate-dt-on-lumi-databridge
branch
from
da7b7e2 to
b2699af
Compare
3 participants
Description
Contributor Declaration
By opening this pull request, I affirm the following:
🌈🌦️📖🚧 Documentation Z3FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/z3fdb/pull-requests/PR-218
🌈🌦️📖🚧 Documentation FDB 🚧📖🌦️🌈
https://sites.ecmwf.int/docs/dev-section/fdb/pull-requests/PR-218