search

MetPX / sarracenia

https://MetPX.github.io/sarracenia
GNU General Public License v2.0
46 stars 22 forks source link

Issue1271 - Fix accomodation to sarra component #1279

Closed andreleblanc11 closed 3 weeks ago

andreleblanc11 commented 3 weeks ago

I originally thought that getData under sarracenia/bulletin.py was correct to fetch data that's not inline, but I was wrong. I don't think it was ever tested.

I've been testing a sarra component on dev with these changes and it looks OK now. It can now get the data through the path.

The goal moving forward is to use this renamer in a non-AM setting through a sarra (which posts to a sender), or a flow/sender that has download True.

github-actions[bot] commented 3 weeks ago

Test Results

238 tests   236 ✅  1m 32s ⏱️   1 suites    1 💤   1 files      1 ❌

For more details on these failures, see this check.

Results for commit 5d6086c8.

:recycle: This comment has been updated with latest results.

petersilva commented 3 weeks ago

There is another routine sarracenia.Message.getContent() in init() that seems to do the same thing as getData... it would be nice to leverage the existing impl, rather than having to maintain 2. but not a big deal. This looks fine.

petersilva commented 3 weeks ago

the PR is marked draft... feel free to merge when you think it is good to go... or if you make substantive chances, ask for another review.

andreleblanc11 commented 3 weeks ago

It doesn't look like that much work and would simplify future code. Currently testing this

andreleblanc11 commented 3 weeks ago

I added some logic so that getContent checks for the inputCharset if it exists. We need this for french chars in bulletins.

andreleblanc11 commented 3 weeks ago

That was my initial attempt at getting options which didn't work. Forgot to remove it after, but isn't causing problems