Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Amda refac and some global refac #17

Merged
merged 38 commits into from
Oct 7, 2021
Merged

Amda refac and some global refac #17

merged 38 commits into from
Oct 7, 2021

Conversation

jeandet
Copy link
Member

@jeandet jeandet commented Aug 6, 2021

This PR is not complete yet, the idea is to:

  • make Timetable and Catalogs related objects as much as possible user friendly with nice repr and UX
  • move Timetable and Catalog object to speazy.common with a dedicated UX which is different from SpeasyVariable
  • remove unnecessary code
  • unify API across providers

@jeandet
Introduced amda.TimetableIndex to replace xmlid a string
9bd9655 
AMDA timetables xmlid are sharedtimeTable_[0:N] which is not much handy
alone.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: some cleanups plus introduced DatasetIndex (same idea than Time…
4c9d893 
…tableIndex)

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: same with CatalogIndex and ParameterIndex
cb41689 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
CDA: Reduce a bit pool size for // tests, this seems to block GH Actions
308fabc 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: Moved config entries to config module and removed SAOP support
0f295e9 
- moving config entries to config module allow user to set and find them
all at the same place
- SOAP module was quite useless and unmaintained and was adding unnecessary
complexity to amda module.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: Now REST client uses http module to ensure useragent is set
650f775 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
ADMA: some tests cleanups
7103d97 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: removed dataty_constructor param
80a69b4 
It was not much used and might lead to weird stuff such as load_timetable
 building something which is not a timetable

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
Moved 429 and 523 HTTP status codes handling to http module
cbd7eb6 
Like this all clients will benefit this feature.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet jeandet added enhancement New feature or request WIP Work In Progress (don't merge) labels Aug 6, 2021
@jeandet jeandet added this to the v1.0.0 milestone Aug 6, 2021
@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2021

This pull request introduces 8 alerts and fixes 8 when merging cbd7eb6 into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information

fixed alerts:

  • 8 for Unused import

@jeandet
New common TimeTable class which fits AMDA needs and can be used with…
c41b381 
… other WS

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 8 alerts and fixes 9 when merging c41b381 into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information

fixed alerts:

  • 8 for Unused import
  • 1 for Module is imported more than once

@jeandet
AMDA: Introduced new Catalog class mostly like TimeTable
12a2173 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@codecov
Copy link

codecov bot commented Aug 9, 2021 •
edited
Loading

Codecov Report

Merging #17 (6754c81) into main (e107171) will increase coverage by 9.94%.
The diff coverage is 87.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
+ Coverage   75.02%   84.96%   +9.94%     
==========================================
  Files          24       31       +7     
  Lines        1085     1450     +365     
  Branches      185      207      +22     
==========================================
+ Hits          814     1232     +418     
+ Misses        232      156      -76     
- Partials       39       62      +23
Flag Coverage Δ
unittests 84.96% <87.75%> (+9.94%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
speasy/core/cache/version.py 100.00% <ø> (ø)
speasy/core/span_utils.py 86.44% <ø> (ø)
speasy/webservices/amda/_impl.py 72.04% <72.04%> (ø)
speasy/webservices/amda/rest_client.py 75.28% <75.28%> (ø)
speasy/core/datetime_range.py 83.72% <83.72%> (ø)
speasy/webservices/cda/__init__.py 46.00% <85.71%> (ø)
speasy/inventory/indexes.py 86.11% <86.11%> (ø)
speasy/webservices/amda/ws.py 86.11% <86.11%> (ø)
speasy/products/dataset.py 87.87% <87.87%> (ø)
speasy/core/__init__.py 89.28% <89.28%> (ø)
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e107171...6754c81. Read the comment docs.

@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 10 alerts and fixes 10 when merging 12a2173 into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information
  • 1 for `__eq__` not overridden when adding attributes
  • 1 for Unused import

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable

@jeandet
AMDA: since get_token is a property token is a better name
7706a86 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: REST client refac
38400fe 
Delegate param handling to requests module and moved endpoints
resolution to send_xxx_request.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
AMDA: rest fix error, thx CI
a64ccc4 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 9, 2021

This pull request introduces 9 alerts and fixes 10 when merging a64ccc4 into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information
  • 1 for `__eq__` not overridden when adding attributes

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable

@jeandet
AMDA: Rest module refac, indirect request basically uses regular requ…
ab932ee 
…est code

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 10, 2021

This pull request introduces 9 alerts and fixes 10 when merging ab932ee into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information
  • 1 for `__eq__` not overridden when adding attributes

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable

@jeandet
AMDA: Fix LGTM legit warning
423b0b5 
Event inherits DateTimeRange which implements __eq__, since Event has
extra members it should override this method.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 10, 2021

This pull request introduces 8 alerts and fixes 10 when merging 423b0b5 into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable

@jeandet
Try to add pytest for tox
5a4975c 
Tox need pytest inside each venv

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
update GH actions
22b47d7 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 10, 2021

This pull request introduces 8 alerts and fixes 10 when merging 22b47d7 into e107171 - view on LGTM.com

new alerts:

  • 8 for Clear-text logging of sensitive information

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable

@jeandet
CONFIG: Adds ability to set entries from ENV + adds tests
140b441 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
LGTM: try to mute false positives
4e0b667 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 12, 2021

This pull request introduces 7 alerts and fixes 12 when merging 4e0b667 into e107171 - view on LGTM.com

new alerts:

  • 7 for Clear-text logging of sensitive information

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
AMDA: forgot to make get_catalog compatible with CatalogIndex
79749fc 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 12, 2021

This pull request introduces 7 alerts and fixes 12 when merging 79749fc into e107171 - view on LGTM.com

new alerts:

  • 7 for Clear-text logging of sensitive information

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
AMDA: Few small refac
2067e3c 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 13, 2021

This pull request introduces 7 alerts and fixes 12 when merging 2067e3c into e107171 - view on LGTM.com

new alerts:

  • 7 for Clear-text logging of sensitive information

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
Fix small mistake on previous variable merge function refac
86d746b 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 15, 2021

This pull request introduces 7 alerts and fixes 12 when merging 86d746b into e107171 - view on LGTM.com

new alerts:

  • 7 for Clear-text logging of sensitive information

fixed alerts:

  • 7 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
Whole API rework and reorganization + doc WIP
80bfdbd 
Webservices are all in webservices package and stuff that was in common
package has been split either in core package for internal API or in
products packages for public API

Added support for doctest and started to rework documentation for better
separation between user and dev doc.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Aug 22, 2021

This pull request introduces 10 alerts and fixes 18 when merging 80bfdbd into e107171 - view on LGTM.com

new alerts:

  • 5 for Conflicting attributes in base classes
  • 3 for Unused import
  • 1 for Wrong name for an argument in a call
  • 1 for Unused local variable

fixed alerts:

  • 13 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

jeandet
jeandet commented Sep 23, 2021
View reviewed changes
tests/test_config_module.py
def tearDown(self):
pass

def test_reads_firt_from_env(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

typo

@jeandet
New user doc WIP plus some cleanups
7e274c9 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Oct 3, 2021

This pull request introduces 10 alerts and fixes 18 when merging 7e274c9 into e107171 - view on LGTM.com

new alerts:

  • 5 for Conflicting attributes in base classes
  • 3 for Unused import
  • 1 for Wrong name for an argument in a call
  • 1 for Unused local variable

fixed alerts:

  • 13 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
[ssc] Fix regression with python 3.6
2d7b6a1 
Looks like one of the latest 3.6 updates broke tz offset parsing.

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
Moved AMDA top module from _amda to ws to make it public for API doc
4527178 
also doctest is python core module

Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
[GH Actions] adds numpydoc
637ce68 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Oct 7, 2021

This pull request introduces 10 alerts and fixes 19 when merging 637ce68 into e107171 - view on LGTM.com

new alerts:

  • 5 for Conflicting attributes in base classes
  • 3 for Unused import
  • 1 for Wrong name for an argument in a call
  • 1 for Unused local variable

fixed alerts:

  • 13 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for Modification of parameter with default
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
[Doc] some cleanups, still a lot to do
8ba0eea 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
[lgtm] fixes few actual errors and mute some false positives
c2d5e21 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@lgtm-com
Copy link

lgtm-com bot commented Oct 7, 2021

This pull request introduces 7 alerts and fixes 19 when merging c2d5e21 into e107171 - view on LGTM.com

new alerts:

  • 5 for Conflicting attributes in base classes
  • 1 for Wrong name for an argument in a call
  • 1 for Unused import

fixed alerts:

  • 13 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for Modification of parameter with default
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet
[GH Actions][lgtm] update codecov action and fix legit issue
43c9864 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet
[Doc] makes REAMDE point to stable documentation
6754c81 
Signed-off-by: Alexis Jeandet <alexis.jeandet@member.fsf.org>
@jeandet jeandet added bug Something isn't working documentation Improvements or additions to documentation and removed WIP Work In Progress (don't merge) labels Oct 7, 2021
@jeandet jeandet marked this pull request as ready for review October 7, 2021 16:07
@lgtm-com
Copy link

lgtm-com bot commented Oct 7, 2021

This pull request introduces 6 alerts and fixes 19 when merging 6754c81 into e107171 - view on LGTM.com

new alerts:

  • 5 for Conflicting attributes in base classes
  • 1 for Unused import

fixed alerts:

  • 13 for Unused import
  • 2 for Module is imported more than once
  • 1 for Unused local variable
  • 1 for Modification of parameter with default
  • 1 for 'import *' may pollute namespace
  • 1 for Clear-text logging of sensitive information

@jeandet jeandet merged commit ae058f3 into SciQLop:main Oct 7, 2021
@jeandet
Copy link
Member Author

jeandet commented Oct 7, 2021

Let's merge this huge PR and switch to a smaller/saner PRs.
Closes #18

@jeandet jeandet mentioned this pull request Oct 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.

None yet
1 participant
@jeandet