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: from-org refactor #1296

Merged
merged 3 commits into from
Apr 23, 2024
Merged

fix: from-org refactor #1296

merged 3 commits into from
Apr 23, 2024

Conversation

mshanemc
Copy link
Contributor

What does this PR do?

  • when checking for undefined filename+type, also check for empty strings
  • delegate connection retry logic to jsforce (removing a dependency!)

What issues does this PR fix or reference?

forcedotcom/cli#2841
@W-15584672@

@mshanemc mshanemc requested a review from a team as a code owner April 23, 2024 16:54
@mshanemc mshanemc changed the title Sm/from org refactor fix: from-org refactor Apr 23, 2024
Comment on lines 31 to 32
private connection: Connection;
private registry: RegistryAccess;
Copy link
Member

Choose a reason for hiding this comment

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

could be readonly if we want to

Comment on lines -166 to -168
// The 'singleRecordQuery' method was having connection errors, using `retry` resolves this
// Note that this type of connection retry logic may someday be added to jsforce v2
// Once that happens this logic could be reverted
Copy link
Member

Choose a reason for hiding this comment

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

Nice

frequency: Duration.milliseconds(1000),
timeout: Duration.minutes(3),
poll: async (): Promise<StatusResult> => {
const res = ensureArray(await this.connection.metadata.list(query));
Copy link
Member

Choose a reason for hiding this comment

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

Have our connections just become more resilient to remove this polling? I don't understand why we're doing this to begin with

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, part of the jsforce stuff that Cristian did jsforce/jsforce#1407.

I think the retry logic was using pollingClient to work around those network issues.

Copy link
Member

Choose a reason for hiding this comment

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

@WillieRuemmele Shane is correct, I added the pollingClient logic here:
#941

jsforce retries on the same errors that pollingClient and more.

@WillieRuemmele
Copy link
Member

QA Notes

Create an org with SynonymDictonary MDT and retrieve

before:

 sf project generate manifest --from-org test-9flhyhvfmlba@example.com                                                  
 ›   Warning: @salesforce/cli update available from 2.36.8 to 2.37.4.
Error (1): _Default: Could not infer a metadata type

Try this:

Additional suggestions:
Confirm the file name, extension, and directory names are correct. Validate against the registry at:
<https://github.com/forcedotcom/source-deploy-retrieve/blob/main/src/registry/metadataRegistry.json>

If the type is not listed in the registry, check that it has Metadata API support via the Metadata Coverage Report:
<https://developer.salesforce.com/docs/metadata-coverage>

If the type is available via Metadata API but not in the registry

- Open an issue <https://github.com/forcedotcom/cli/issues>
- Add the type via PR. Instructions: <https://github.com/forcedotcom/source-deploy-retrieve/blob/main/contributing/metadata.md>

✅ : after

 ➜  ../../oss/plugin-deploy-retrieve/bin/run.js project generate manifest --from-org test-9flhyhvfmlba@example.com 
Warning: _Default: Could not infer a metadata type
successfully wrote package.xml

NOTE: package.xml does not contain the SynonymDictonairy entry

@WillieRuemmele WillieRuemmele merged commit 2fecb51 into main Apr 23, 2024
68 checks passed
@WillieRuemmele WillieRuemmele deleted the sm/from-org-refactor branch April 23, 2024 21:57
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.

4 participants