Skip to content

Refactor the service layer#79

Merged
BytecodeBrewer merged 5 commits into
developfrom
refactor-the-service-layer
Jul 7, 2026
Merged

Refactor the service layer#79
BytecodeBrewer merged 5 commits into
developfrom
refactor-the-service-layer

Conversation

@BytecodeBrewer

@BytecodeBrewer BytecodeBrewer commented Jul 7, 2026

Copy link
Copy Markdown
Owner

What changed?

  • dissolve calculator and conversion service
  • update exchange clients test

Related issue

Closes #71

Tests

  • I ran pytest
  • Existing tests pass
  • I added or updated tests where necessary
  • Not needed because this only affects documentation or repository setup

Checklist

  • The change is focused
  • No secrets or API keys are included
  • Documentation was updated if needed
  • CI is expected to pass

Notes

  • methods from previosly conversion service aren't tested yet -> before the sprint 3 starts and CD Pipleline is going o be implemented, its need to be done.

@BytecodeBrewer BytecodeBrewer merged commit ea8f82b into develop Jul 7, 2026
8 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 17cfee10c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/argus/gui/app.py
Comment on lines +93 to +94
source = DataSource(name="", provider_kind="")
instrument = Instrument(symbol="EUR/USD", name="EUR - USD", asset_class="fx")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Populate the trend request with usable FX metadata

When the Trend Chart is opened on a fresh database, this request is the only thing used by prepare_trend_analysis. read_price_bars searches by source.name and instrument.symbol, so name="" won't match rows inserted under real sources, and on a miss yfinance_client.get_timeseries constructs its ticker from instrument.base_currency and quote_currency, both left as None here. The result is a NoneNone=X lookup and no chart for EUR/USD instead of fetching/caching the intended pair.

Useful? React with 👍 / 👎.

Comment on lines +32 to 33
if 0 == 0:
return None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore the API lookup for legacy conversions

After interface.py was switched to import legacy.services.conversion_service, every legacy CLI conversion reaches this get_conv_rate. This unconditional branch returns None for all currency pairs, so display_convert never succeeds and stays in the retry loop even for valid inputs that used to call exchangerate_client.get_rates. Please keep the real rate lookup when moving the service.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant