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.

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

Add ServerContext #606

Merged
merged 24 commits into from
Nov 10, 2022
Merged

Add ServerContext #606

merged 24 commits into from
Nov 10, 2022

Conversation

cbellot000
Copy link
Contributor

@cbellot000 cbellot000 commented Nov 8, 2022

Gives the ability to choose the context with which the server should be started.
The context allows to choose which capabilities are available.

xml_path : str, optional
Path to the xml to load.
"""
def __init__(self, context_type=EContextType.user_defined, xml_path=""):
Copy link
Contributor

Choose a reason for hiding this comment

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

@cbellot000 Why is the user_defined context the default one? I think this makes the xml_path non-optional in a way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PProfizi it's the default of this constructor (because you would only need to use the constructor directly when you want to speicify an xml), it doesn't make user defined the default context (which is SERVER_CONTEXT = AvailableServerContexts.entry)

self._base_service.apply_context(server_context.SERVER_CONTEXT)
except errors.DpfVersionNotSupported:
self._base_service.initialize_with_context(
server_context.AvailableServerContexts.premium
Copy link
Contributor

Choose a reason for hiding this comment

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

@cbellot000 Why premium in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PProfizi premium was the old default (equivalent to standalone)

tests/conftest.py Outdated Show resolved Hide resolved
tests/test_multi_server.py Outdated Show resolved Hide resolved
@@ -198,6 +201,10 @@ def cyclic_multistage():
SERVERS_VERSION_GREATER_THAN_OR_EQUAL_TO_3_0 = meets_version(
get_server_version(core._global_server()), "3.0"
)
SERVERS_VERSION_GREATER_THAN_OR_EQUAL_TO_2_0 = meets_version(
Copy link
Contributor

Choose a reason for hiding this comment

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

@cbellot000 Is the minimum server version required 2.0 or 2.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's how it was used before

tests/test_service.py Outdated Show resolved Hide resolved
tests/test_service.py Outdated Show resolved Hide resolved
cbellot000 and others added 13 commits November 8, 2022 11:46
Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
Co-authored-by: PProfizi <100710998+PProfizi@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #606 (79ab562) into master (be8d22d) will increase coverage by 0.07%.
The diff coverage is 87.87%.

@@            Coverage Diff             @@
##           master     #606      +/-   ##
==========================================
+ Coverage   87.90%   87.98%   +0.07%     
==========================================
  Files          68       69       +1     
  Lines        7566     7630      +64     
==========================================
+ Hits         6651     6713      +62     
- Misses        915      917       +2     

@cbellot000 cbellot000 merged commit 3571fa5 into master Nov 10, 2022
@cbellot000 cbellot000 deleted the feature/expose_context branch November 10, 2022 09:27
@PProfizi PProfizi changed the title Feature/expose context Add ServerContext Nov 25, 2022
@PProfizi PProfizi added the enhancement New feature or request label Nov 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants