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

Fix profile merge order to match Node.js SDK #190

Closed
t1m0thyj opened this issue Jun 19, 2023 · 1 comment · Fixed by #203
Closed

Fix profile merge order to match Node.js SDK #190

t1m0thyj opened this issue Jun 19, 2023 · 1 comment · Fixed by #203
Assignees
Labels
enhancement New feature or request priority-high

Comments

@t1m0thyj
Copy link
Member

t1m0thyj commented Jun 19, 2023

Tested #144 with the sample config and profiles that we've been using. For the my_zosmf profile it is failing to load user and password:
Python SDK: {'host': 'example.com', 'rejectUnauthorized': True, 'port': 443}
Node.js SDK: {"port":443,"host":"example.com","rejectUnauthorized":true,"user":"admin","password":"password"}

The ProfileManager.load method loops through config files in the following order:

  1. Project User Config (./zowe.config.user.json)
  2. Project Config (./zowe.config.json)
  3. Global User Config (~/zowe.config.user.json)
  4. Global Config (~/zowe.config.json)

In the current implementation, profile properties are first loaded from each layer of the config individually, and merged together afterwards.

For the case of our sample config:

  • The user and password properties are stored in the my_base profile of Project User Config (./zowe.config.user.json).
  • This my_base profile is defined as the default base profile in Project Config (./zowe.config.json).
  • At the time when we load Project User Config, we do not yet know that my_base is the default base profile, so the current logic will skip loading its properties. 😢

To fix this, I believe refactoring ProfileManager.load will be necessary. We should first merge all the config layers together, and then load profile properties as a single operation from the resulting merged object.

For a detailed explanation of how config layers should be merged together, see Zowe docs.

Originally posted by @t1m0thyj in #144 (comment)

@t1m0thyj t1m0thyj added enhancement New feature or request priority-high labels Jun 19, 2023
@t1m0thyj t1m0thyj added this to the Zowe v2 Team Configuration milestone Jun 19, 2023
@github-actions
Copy link

Thank you for raising this enhancement request.
The community has 90 days to vote on it.
If the enhancement receives at least 5 upvotes, it is added to our development backlog.
If it receives fewer votes, the issue is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority-high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants